ollayf / pe

0 stars 0 forks source link

Exclusion of some important classes in the sequence diagram for simplicity #16

Open ollayf opened 1 year ago

ollayf commented 1 year ago

Should the dish also not be added into the storage? Why is the whole connection to the XYZStorage classes not included as they are important. I want to know how the StorageComponent works

Screenshot 2023-04-14 at 5.51.59 PM.png

nus-pe-bot commented 1 year ago

Team's Response

This component is talking about the missing storage part in the sequence diagram. It is similar to the issue #1018. Hence, it is tagged as duplicate.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

Clearer overview of how parts of the architecture work together in the main functions

Note from the teaching team: This bug was reported during the Part II (Evaluating Documents) stage of the PE. You may reject this bug if it is not related to the quality of documentation.


Since there are 4 separate system seem to be happening for staff, dish, and the rest. It would be good if you could go indepth in the overarching idea of what happens in a delete, add, view and search functions the current documentation does not show how it goes into storage (in memory) and pushed into the output files respectively. For example, here, I do not know much more than the fact that it execute the addMeeting function. What does that function do? how does it relate to other functions. If I pull out other sequence diagrams I would face the same issue

Screenshot 2023-04-14 at 5.47.48 PM.png


[original: nus-cs2113-AY2223S2/pe-interim#1375] [original labels: severity.Medium type.DocumentationBug]

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

To avoid cluttering the diagram and repeating certain sections of the DG again, we had explained the storage component in the another section of the DG. The XYZStorage class is a generalisation of how the storage system for all the features work. As you can see here, the manager class of each feature will call their respective storage class. Furthermore, the main focus of the section mentioned is to explain how addXYZ works. Adding in the parts about storage component would dilute the purpose of the diagram.

image.png

Thus, we think that this is not a bug and reject this issue.

Items for the Tester to Verify

:question: Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

Reason for disagreement: [replace this with your explanation]


## :question: Issue response Team chose [`response.Rejected`] - [x] I disagree **Reason for disagreement:** Again with all these issues group together... It does not tell me much. THe screenshotted diagrams have only 3-4 function calls across 2-3 classes. If I wanted to understand your function, say XYZManager SD that only calls 2 other functions and 1 other class, I will jsut read your code directly which give me more info. The sequence diagram is supposed to give me a high level view of the connectivity. While I can agree they are duplicated. This cannot be rejected as an issue to me.