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

Phase management, transition handling, and command validation #94

Closed

Conversation

geoboom
Copy link

@geoboom geoboom commented Oct 20, 2020

Hey everyone, I added phase logic :o

Here are some new files:

src/main/java/seedu/address/logic/phase
├── exceptions
│   └── PhaseInvalidException.java   <- exception thrown if the command not permitted in current phase
├── Phase.java          <- enum of Phases
└── PhaseManager.java   <- manages the phases and phase transitions 

I also augmented the Command class by adding a nextPhase attribute which lets PhaseManager know what phase to transition (if any) to upon execution of the command. I also added a allowedPhases hash set to let the PhaseManager check whether a given command is permitted to execute in the current phase. If not, then PhaseInvalidException will be thrown that ideally bubbles up to the UI manager and alerts the user ^_^

I updated LogicManager to contain the above checking and exception logic (no pun intended).

Lastly, I have written a test PhaseManagerTest which as of now tests very little. Still figuring out how to mock a Command object so I can test for correct transitions and whether exceptions are thrown correctly. Maybe someone can point me in the right direction as I work on other features 🙏.

@geoboom
Copy link
Author

geoboom commented Oct 20, 2020

Weird... CI not passing even after I resolved the issues with not catching PhaseInvalidException. Will fix after I finish data analytics features and stuff.

@MelanieNPS
Copy link

About the test for the command object, you can try refering to the tests for the other commands. They check if the command result given by the command being executed matches with the expected command result

@GabrielSimbingyang
Copy link

Will not be including in final version of GreenTea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants