williamlamjy / pe

0 stars 0 forks source link

No diagram seq or class for Storage class and processes #31

Open williamlamjy opened 2 years ago

williamlamjy commented 2 years ago

This chunk of text would be a lot clearer with there is seq diag to show the process of memory storage yet there is no diagram here.

Screenshot 2021-11-12 at 5.40.42 PM.png

nus-se-script commented 2 years ago

Team's Response

We believe we dont need to provide seq/class diagrams for every single method/class since each of us were only required to add in 1-2 diagrams each. We followed the AddressBook-Level3 (AB3) DG structure, and similarly, that isnt comprehensive of every single feature/class.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: This is simply untrue. AB3 may not go through every class but it goes through every component which is inclusive of storage Screenshot 2021-11-18 at 9.54.07 AM.png While it does not show implementation/seq diagram for storage, it has a class diagram for storage for extra clarity. Completely omitting a whole component in your code is not optimal for new developers/readers. This chunk of text with a whole bunch of typos would be a lot clearer with a sequential diagram which is undeniable so why not implement it? Or maybe at least a class diagram.


:question: Issue severity

Team chose [severity.VeryLow] Originally [severity.High]

Reason for disagreement: Severity very low indicates that it is merely appearance that is affected. As a developer every piece of code runs through the storage component so it would help a lot if a sequential/class diagram is included and this chunk of text would be an inconvenience to new developers coming into the team. Especially if they are adding a new component to your software, they would have to include the corresponding encoder/decoder functionalities in your storage class so how exactly do they implement this if there is no absolutely no guidance in this field other than this short paragraph?