nus-cs2103-AY2425S1 / pe-dev-response

0 stars 0 forks source link

Incorrect arrowhead usage in sequence diagram in developer guide #1098

Open nus-se-script opened 1 week ago

nus-se-script commented 1 week ago

Issue: open arrowheads are used instead of filled arrowheads to represent synchronous method calls.

From 2103 website: image.png

From DG: image.png

Rationale: The sequence diagrams in the developer guide incorrectly use open arrowheads, which are intended for asynchronous method calls, to represent synchronous calls. This flaw leads to significant confusion for developers, especially new or onboarding team members, who rely on these diagrams to understand the system's architecture and interactions. For some developers, the misrepresentation of method call types could lead to incorrect assumptions about the application's behavior, resulting in wasted time verifying code to determine if calls are actually synchronous or asynchronous. This inconsistency not only causes occasional inconvenience but also undermines the purpose of the diagrams, rendering them almost unusable for developers seeking accurate guidance. For users relying solely on the diagrams without checking the code, this could lead to major problems, incorrect implementations, or delays in understanding system functionality. As a result, the value of the developer guide is significantly diminished, particularly for new team members, who may ultimately abandon these diagrams as a resource. Updating the diagrams to use the correct arrowheads for synchronous calls would ensure accuracy and restore the usefulness of the developer guide for all developers. After a careful assessment of this issue’s impact, I have assigned it a medium severity. Although the incorrect use of asynchronous arrowheads in the sequence diagrams significantly disrupts usability for developers, particularly new team members, and could justify a higher severity due to the confusion and potential misinterpretations it causes, I believe marking it at this level should allow us to reach a consensus and support our shared goal of improving the accuracy and accessibility of the developer guide.


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

Chronoxy commented 1 week ago

Team's Response

Thank you for your issue.

Furthermore, difference between PlantUML convention and textbook convention

From this source, we are able to see a clear distinction between the filled arrowheads (asynchronous) and lined arrowheads (synchronous) for PlantUML convention. However, if we were to follow our textbook convention, filled arrowheads are used for synchronous and lined arrowheads are used for asynchronous. Hence despite being different from PlantUML convention, we are adhering to the textbook convention, utilizing filled arrowheads for synchronous calls. Which in this case, we believe we are correct. image.png taken from: https://plantuml-documentation.readthedocs.io/en/latest/diagrams/sequence.html as compared to: image.png

Therefore, we concur to reject this issue. Thank you

Duplicate status (if any):

--