[CS2113T-F14-2] Fitbot#50
Open
tryyang2001 wants to merge 887 commits intonus-cs2113-AY2122S1:masterfrom
Open
Conversation
tngjunwei
reviewed
Oct 28, 2021
tngjunwei
left a comment
tngjunwei
left a comment
There was a problem hiding this comment.
Overall, the developer's guide is very well written. The explanations provided makes it very easy to understand. The minor issues only lie with the diagrams. Keep it up!
Comment on lines
+55
to
+58
| Interaction between the classes could be shown by the uml sequence diagram below. | ||
|
|
||
| <p align="center" width="100%"> | ||
| <img width="100%" src="images/Architecture.png" alt="Architecture Sequence Diagram"/> |
There was a problem hiding this comment.
There seems to be a operation invocation missing.
Comment on lines
+62
to
+65
| -When there is an input, the Ui class will retrieve the information from the user. | ||
| -Once the Main class receives the input, it creates a new Parser class to parse the commands. | ||
| -Depends on the method, in this case add food command, main class will execute the command class(not shown) | ||
| based on the command the parser detects. |
There was a problem hiding this comment.
Slight markdown format bug - need to insert space between dash and the starting word
Comment on lines
+168
to
+171
|
|
||
| <p align="center" width="100%"> | ||
| <img width="100%" src="images/LogicSequenceDiagram.png" alt="Logic Sequence Diagram"/> | ||
| </p> |
There was a problem hiding this comment.
I think this has the same issue as above - missing method invocation.
Comment on lines
+217
to
+218
|  | ||
|
|
There was a problem hiding this comment.
Perhaps you can replace the alt frame with an opt frame instead? This way, you won't repeat the constructor call.
Author
There was a problem hiding this comment.
Sure! The sequence diagram has been updated! Thanks for pointing this out :D
Comment on lines
+310
to
+331
| #### **_dependency_** | ||
| In UML diagram, dependency is a directed relationship which is used to show that some elements or a set of elements requires, | ||
| needs or depends on other model elements for specification or implementation. | ||
| #### **_superclass_** | ||
| A class from which other classes inherit its code. The class that inherits its code will be able to access some/all | ||
| functionalities from the superclass. | ||
| #### **_subclass_** | ||
| A class that inherits code from the other classes. Such class will be able to access some/all functionalities from its superclass, | ||
| but not vice versa. | ||
| #### **_abstract class_** | ||
| A class that cannot be created using constructor. Usually such class is a superclass, and it does not give meaningful | ||
| value if one tries to construct it. | ||
| #### **_self invocation_** | ||
| In UML sequence diagram, a method that does a calling to another of its own methods is called self-invocation. | ||
| #### **_singly linked list_** | ||
| A linear data structure that behaves like an array except that the elements inside linked list is not store at a contiguous | ||
| location. In Java, linked list can be implemented using `ArrayList` in `Collection`. | ||
| #### **_priority queue_** | ||
| An abstract data type similar to a regular queue or stack data structure in which elements in priority queue are ordered | ||
| and have "priority" associated with each element. The priority can be defined by the coder. In the case of `FoodList`, the | ||
| priority will be defined as earlier date and time will have higher priority. | ||
| (more coming in the future...) |
There was a problem hiding this comment.
Since this is a developer guide, maybe it won't be necessary to include a glossary for these technical terms?
I think this section is more for clarifying the meaning of certain potentially ambiguous terms.
Author
There was a problem hiding this comment.
OK! We will consider this suggestion :) Thank you!
Comment on lines
+70
to
+74
| ### Data Component (Profile) | ||
|
|
||
| <p align="center" width="100%"> | ||
| <img width="90%" src="images/ProfileClassDiagram.png" alt="Architecture Sequence Diagram"/> | ||
| </p> |
There was a problem hiding this comment.
Perhaps you can list down the objects aggregated in Profile instead? There seems to be a lot of objects, and I am not sure how a class diagram will help here.
Comment on lines
+175
to
+180
| ### Storage component | ||
|
|
||
| <p align="center" width="100%"> | ||
| <img width="90%" src="images/StorageManagerClassDiagram.png" alt="Architecture Sequence Diagram"/> | ||
| </p> | ||
|
|
There was a problem hiding this comment.
Similar to the Data component section, perhaps you could list down the interfaces StorageManager implements, etc?
refactor isValid method move getActualIndex to ItemList and implement code inheritance there
implement isValid method in Food and Exercise
Make profile even more OOP
…ect is deleted update manual testing section (add in instructions for view and delete)
…on, changed \n to LS update userguide accordingly. Repeat user message on invalid input to prevent raging users.
…into Branch-V2.1
Update DG and Item Bank and Item Class Diagram
…into v2.1 # Conflicts: # src/test/java/seedu/duke/food/FoodListTest.java
# Conflicts: # src/test/java/seedu/duke/food/FoodListTest.java
* master: delete outdated junit test in FoodListTest update input code example in Add Exercise command add explanation for why the lifeline does not end even though the object is deleted update manual testing section (add in instructions for view and delete) update puml diagram add Interface to the class diagram Minor change to accessibility in code Add enum class for activity levels Add ProfileUtils class to make profile more OOP fix spacing for javaDoc in itemlist line 172 to pass check implement isValid method in Food and Exercise refactor isValid method move getActualIndex to ItemList and implement code inheritance there Update data integrity of food and exercise Fix formatting error in delete command
Refactor StartState
Developer guide updates to Storage and Profile
Format PPP
Fix typo and formatting issue in DG
Update PPP
Update UG and fix bugs for release
Shorten PPP
format example in UG
second update to ug
Update diagrams
Change diagrams colour
add default calorieGoal if calorieGoal is not initialised
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fitbot is a desktop app that helps university students who are looking to keep track of their fitness and health with the speed and convenience of command-line based tools, especially in times of online school.