AY1920S2-CS2103T-W12-1 / main

Delino - Managing Delivery Tasks
https://ay1920s2-cs2103t-w12-1.github.io/main/
MIT License
0 stars 6 forks source link

W10 Tutorial DG Review #210

Open J-Dan23 opened 4 years ago

J-Dan23 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.