DG Comments

[hughjazzman]

DG Comments I

• (Architecture Sequence Diagram) The lifeline in this diagram could be properly ended - can be fixed in PlantUML.

• (Data Component Class Diagram) The Main and Logic "classes(?)" in this diagram could be removed to reduce clutter. The relationship between the classes could perhaps be what the diagram is showcasing. #149

• (Logic Class Diagrams) @tlyi Ultimately the diagram is probably fine - maybe you could add a note explaining which classes are purely utility classes?

• (Logic Sequence Diagram) Perhaps the lifeline should continue between the return from parseCommand() and execute()?

DG Comments II

• (Architecture Sequence Diagram) Consider reworking this diagram. After control is returned to :Main, I believe it looks as follows:

1. :Main calls a :Ui method - should be a solid arrow
2. Control returns to :Main - dotted arrow, control does not go to the User yet
3. saveFoodList is carried out
4. Control returns to :Main
5. Control returns to :Main from :Ui
6. Control returns to :Ui to User
   Essentially the control is not returned to the User so early - the diagram shows control belonging to 2 entities at the same time.
   

• (ItemBank Class Diagram) As mentioned in Tutorial, the dependencies to {abstract} Item may already implied by the inheritance. #179

• (Logic Sequence Diagrams) Consider breaking the diagram down even more to provide clarity.
• (All Sequence Diagrams) Consider explaining why the lifelines do not end at the cross.

[tlyi]

Hi @hughjazzman, in regards to the comment "The relationships between the classes could perhaps be clearer on whether they should be associations or dependencies?", may I know how do I represent static method/constant calls in another class? For e.g, ParserManager does not contain any object from ParserMessages, it only refers to a constant variable there. XYZCommandParser also does not contain any object from ParserUtils, and only calls static methods like ParserUtils.extractXXXX(). Are these supposed to be represented as associations or dependencies?

[hughjazzman]

I've edited above to the latest DG as of today.