eunrcn / pe

0 stars 0 forks source link

Lack of activity, class, sequence diagrams for each command created #17

Open eunrcn opened 3 months ago

eunrcn commented 3 months ago

My deepest pologies but seems like there is only 1 sequence diagram for borrowing a book, but adding book returning book, donating book are missing their respective activity, class and sequence diagrams.

Things that would benefit a developer -at least one class diagram for any new commands yall have created, so i can understand the logic behind the new commands -sequence diagram for borrow book was well done but without its class diagram, it is difficult to piece everything together -at least one activity diagram for any new commands yall have created, so i can have a visual representation of the logic behind the new commands

Screenshots include explanation and chunks of text with no visuals like activity, class, sequence diagrams

Screenshot 2024-04-19 at 5.37.36 PM.png

Screenshot 2024-04-19 at 5.31.17 PM.png

nus-se-bot commented 3 months ago

[IMPORTANT!: Please do not edit or reply to this comment using the GitHub UI. You can respond to it using CATcher during the next phase of the PE]

Team's Response

No details provided by team.

The 'Original' Bug

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

insufficient diagrams present in the implementation part of the dg

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.


Expected Behavior

As a developer working on the application. It would be great for diagrams to be included especially when discussing about the implementation of the respective application feature. This could involve sequence diagrams so that I would know how the entities within the system communicate / interface with each other to carry out the feature. Activity diagrams would be great as well so as to know the general workflow of the feature.

Actual Behavior

Currently, it is mainly plain text explaining the implementation which might require much more cognitive load and result to the developer having a slower onboarding process.

image.png


[original: nus-cs2103-AY2324S2/pe-interim#478] [original labels: severity.Low type.DocumentationBug]

Their Response to the 'Original' Bug

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

Thank you for pointing out the bug!

We have actually deliberately used less diagrams due to having unnecessary bloat of having too many diagrams.

As of our current Developer Guide, prior to the Implementation section, our Design section has a sequence diagram to describe how a Command interacts with Logic, as well as a separate sequence diagram to show how a Command that affects Book interacts with Library in Model.

As in general, the flow of the most of the commands can be explained via these 2 diagrams (with the only differences being the methods called), we believe that putting a diagram for every section in Implementation is repeating a lot of duplicate information and creates bloat in information.

However, we do see your concerns for the higher cognitive load resulting in the developer having a slower onboarding process, hence we do agree that we could perhaps include activity diagrams in the individual sections. However, we think that this is not that severe of an issue as we have already formatted it such that each section is broken down into smaller subsections for easier reading and is only for the ease of the user, hence we have left it as Low severity.

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 reason]


## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.High`] - [ ] I disagree **Reason for disagreement:** [replace this with your reason]