TheDinos / pe

0 stars 0 forks source link

Sequence Diagram has multiple return arrows for one function call. #6

Open TheDinos opened 1 week ago

TheDinos commented 1 week ago

image.png

In the sequence diagram shown above, several function calls have multiple return "dotted" arrows, which is incorrect by convention.

Example 1:

image.png

In the above image, the developers are trying to show that the showCommandSuccessMessage() call causes a success message to be printed to the user by the Ui, before returning to the Command class. However, a dotted arrow is used to indicate the success message is being printed to the Ui. Suggestion: Instead of a dotted arrow, perhaps a comment box could be placed inside the activation box to indicate that the success message is being printed instead.

Example 2:

image.png

Similar to the first example, developers are trying to show that the showErrorMessage() function call causes an exception message to be printed to the user by the Ui. Perhaps a comment box (like above) could be used to indicate the exception message being printed instead.

nus-se-script commented 5 days ago

Team's Response

The team believes that this is a matter of non-standard notation, rather than an incorrect notation (ie. not defined by the course textbook). Based just on the method calls in this diagram, the arrows are correct, as the user is just another entity. We have also lowered the severity as it does not hinder understanding.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: On the contrary, I believe that it has been defined by the course textbook, as displayed in the images below:

image.png

image.png

The textbook states that the dotted arrows are meant to represent method returns. Each of the method calls raised in the initial issue are supposed to have only one return call, and hence only have one return dotted arrow. Hence, it is not a case of non-standard notation.


## :question: Issue severity Team chose [`severity.VeryLow`] Originally [`severity.Low`] - [x] I disagree **Reason for disagreement:** Based on the standard notation as indicated by the textbook (as mentioned for above), it could mislead the developer into thinking that the method has multiple return calls. It is hence not merely a cosmetic issue, and perhaps should be looked at as a severity low.