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

W10 Tutorial DG Review #210

Open
J-Dan23 opened this issue Mar 27, 2020 · 5 comments
Open

W10 Tutorial DG Review #210

J-Dan23 opened this issue Mar 27, 2020 · 5 comments

Comments

@J-Dan23
Copy link

J-Dan23 commented Mar 27, 2020

Setting up:
image
Under 3.: Still named as "seedu.address.Main" Should it be renamed to match your morphed application? Check if there are other yet to be renamed path names.

General comments:
For consistency reasons, check if any of your diagrams are missing figure labels/descriptors.
Nice, consistent use of case for the headers. Good job!
Bold markdown words might be more suitable as code markdowns instead, such as postal sector could be postal sector instead.
Some figures are lacking names, while other have them.
Subsectioning and hyperlinks make the DG very easy to navigate.

@J-Dan23
Copy link
Author

J-Dan23 commented Mar 27, 2020

Design:

2.1
image
Bug in Fig 3: Should the return path from :Storage to Model for [when deleting return order] be outside the alternate path box?

2.1
image
Bug: Should PersonListPanel be mentioned in the description? There might be similar bugs in subsequent sections where components and methods are not renamed.

@J-Dan23
Copy link
Author

J-Dan23 commented Mar 27, 2020

2.4. Model

image
For consistency reasons, headers should have the same format. In this case do you plan to call them all "The xxx component," or simply "The xxx,"?
Additionally, if "Order Book" is an object, should it be marked down? Check if there are other similar mark down issues in your DG.

image
Bug: BetterModelClassDiagram not displaying.

@J-Dan23
Copy link
Author

J-Dan23 commented Mar 27, 2020

2.2. Storage Component

image
For consistency reasons, should Fig 8 also be surrounded by a box labelled "Storage" as well? Check if there are other diagrams similarly lacking in component boxes.

@J-Dan23
Copy link
Author

J-Dan23 commented Mar 27, 2020

3.1. Delivered feature

image
Our team greatly appreciates the class diagram. However, it might be even better to make use of your class diagrams to explain how a feature works (in this case, the delete feature).
You may want to make the notation adhere to UML notation, though, to make it even more readable.

image
Odd line length formatting here.

@J-Dan23
Copy link
Author

J-Dan23 commented Mar 27, 2020

3.2 Nearby feature

image
The class diagram can be a little overwhelming. Thus, maybe consider simplifying both the diagram and the code by using a hashmap of variables with the key being the district number perhaps? Alternatively, you could collate the variables using "districtN" or "addDistrictN".
In summary, perhaps an architecture diagram might be more useful here, as the information would be more concise and clearer due to reduced stating of methods and variables.

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

No branches or pull requests

1 participant