Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Separate UI and Config Modules #53

Open
4 tasks
gregbell26 opened this issue Jan 24, 2023 · 1 comment
Open
4 tasks

[BUG] Separate UI and Config Modules #53

gregbell26 opened this issue Jan 24, 2023 · 1 comment
Labels
bug Something isn't working good first issue Good for newcomers Module: Config This issue relates to the Config module Module: UI This issue relates to UI need analysis review is needed

Comments

@gregbell26
Copy link
Member

Description

Issue

There is a lot of business logic currently mixed with the UI in the Config module. This is a contributing factor to the mess that is the config module.

Steps to recreate

N/A

Solution

Separate the UI and business logic in the config module. config.newConfigFile is the most egregious example of this, however many of the methods in this module exhibit similar issues.

Should move business logic into separate, unit-testable, methods and move the UI into the UI module. See UI.standardGrading for an example of how this was done in the past.

Ideally, all user input should be handled by the uiHelpers.getUserInput method.

The other major thing for consideration is how we want to read the config. Currently, we are doing it with a single call to config.readConfig in the main method, which handles the reading and selection in the event that there are many config files. I also want to separate the business logic (getting the list of available configs, reading the selected one) and the UI (selecting the config file).

Acceptance Criteria

  • Business logic is broken out into separate methods as needed.
  • Business logic is unit testable.
    • Note: Don't write unit tests for this yet, that will be a separate issue once the changes are discussed in more detail.
  • UI is completely moved into UI module.
  • All user input calls utilize uiHelpers.getUserInput
@gregbell26 gregbell26 added bug Something isn't working good first issue Good for newcomers need analysis review is needed Module: UI This issue relates to UI Module: Config This issue relates to the Config module labels Jan 24, 2023
@github-project-automation github-project-automation bot moved this to New Feature in Grading Script Jan 24, 2023
@gregbell26 gregbell26 moved this from New Feature to Bug in Grading Script Jan 24, 2023
@gregbell26
Copy link
Member Author

Wait for #47 to be resolved before working on this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers Module: Config This issue relates to the Config module Module: UI This issue relates to UI need analysis review is needed
Projects
None yet
Development

No branches or pull requests

1 participant