-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update DeveloperGuide with Switch Mode implementation #98
Update DeveloperGuide with Switch Mode implementation #98
Conversation
Codecov Report
@@ Coverage Diff @@
## master #98 +/- ##
============================================
- Coverage 50.51% 50.30% -0.21%
- Complexity 609 610 +1
============================================
Files 141 141
Lines 2647 2664 +17
Branches 303 307 +4
============================================
+ Hits 1337 1340 +3
- Misses 1213 1224 +11
- Partials 97 100 +3
Continue to review full report at Codecov.
|
docs/DeveloperGuide.md
Outdated
|
||
#### Implementation | ||
|
||
Productiv can be in different modes, such as dashboard, deliverable, meeting and contact mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minor, but maybe can use 'specifically' or 'different modes: dashboard, deliverable,...'
docs/DeveloperGuide.md
Outdated
![SwitchModeSequenceDiagram](images/SwitchModeSequenceDiagram.png) | ||
Figure <?> Switch Command Sequence Diagram (In Progress) | ||
|
||
The user input is passed to LogicModeManager. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LogicModeManager
docs/DeveloperGuide.md
Outdated
* Cons: May violate Single Responsibility Principle. | ||
|
||
* **Alternative 2:** Store mode in a `LogicModeManager` | ||
* Pros: Seems to follow the Single Responsibility Principle better. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe can be assertive and say "Aligns with / Follows the Single Responsibility Principle better"
|
||
* **Alternative 2:** Store mode in a `LogicModeManager` | ||
* Pros: Seems to follow the Single Responsibility Principle better. | ||
* Cons: `LogicModeManager` would need to have references to the other logic managers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, but LogicManager
s ?
SwitchCommandParser --> ModeParser | ||
deactivate SwitchCommandParser | ||
|
||
ModeParser --> LogicModeManager : s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe can use a more descriptive name than 's'?
LogicModeManager --> MainWindow : result | ||
deactivate LogicModeManager | ||
|
||
MainWindow -> MainWindow: switchMode(mode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, but is mode the same as result?
No description provided.