search

PresleyChew / pe

0 stars 0 forks source link

[DG] Incorrect notation used for sequence diagram page 9 #6

Open PresleyChew opened 1 week ago

PresleyChew commented 1 week ago

Background

While the forward arrow can be used for Operation invoked and the mention of Informal operation descriptions such as those given in the example below can be used, if more precise details are not required for the task at hand. , it is not used for if conditions. Here the forward arrow shows add if it is not a duplicate . We should follow the notation taught in lecture if there is an optional path instead of trying to cram it into forward arrow meant for operation invoked.

To further support, this cramming if conditions into forward arrow for operation invoked is incorrect as the return arrow may or may not return as the call may or may not happen.

Screenshot 2024-11-15 at 5.11.13 PM.pngScreenshot 2024-11-15 at 5.11.13 PM

Screenshot 2024-11-15 at 5.09.21 PM.png Screenshot 2024-11-15 at 5.09.21 PM

image.png

Reason for Severity

Standard UML error severity as per lecture notes

Possible fix

Use proper optional path convention or consider using reference frame if it is too big.

nus-se-bot commented 1 week ago

Team's Response

No details provided by team.

The 'Original' Bug

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

Wrong format for optional paths for sequence diagram

This is the sequence diagram shown for AddCommandParser:

image.png

The arrow to model states "add if not duplicate", signifying an optional path which is only taken if user does not add a duplicate. However, this does not follow the correct notation for optional paths, which is shown below:

image.png

This makes it possibly confusing for the reader.

Perhaps we could follow this format instead?

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

Their Response to the 'Original' Bug

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

We reject this for the following reason(s):

  1. It is mentioned here that using pseudocode for conditional statements does not violate any notational rules.
  2. We omitted low-level details to make our diagrams easy to understand for the reader, which warrants our use of pseudocode. Moreover, using the opt block will bring in low-level implementation details such as exception handling (for duplicates) which we do not intend to show as it may further confuse the reader. The omitting of these details also does not cause the reader to miss out on any important information.

    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:** Thanks for the response, Prof did say that "you still need to decide whether this is better than the other alternatives. For example, if you think omitting the more visual opt frame can mislead the reader or increase the risk of the reader missing an important information, it can still be considered as a defect." The sequence diagram in question is illustrated to let readers have a clearer picture as to how adding a new internship application would look like in terms of the interactions. I believe that the "adding if not duplicate" SHOULD be shown and can be done so via an optional pathway, You do not have to expose lower level implementation like exceptional handling if duplicate is detected by simply having reference frames (which is warranted to be omitted as the diagram is meant to show adding of NEW internship application). However omitting the actual adding is an erroneous decision as that is one of the main point of the diagram. As a reader I have no idea how your logic and model work because you just left a big portion of important interactions onto 1 operation invoked arrow between Logic and Model, defeating the purpose of the actual UML diagram and I would even argue rendering it obsolete. Given that this diagram is used to show the adding of new internship application, I would argue that as a reader I am missing an important information as what Prof mentioned as I have no idea how "add if it is not a duplicate" works. Perhaps if the diagram is NOT used for this specific context, then this syntax would be acceptable.