kaixin-hc / pe

0 stars 0 forks source link

Fork syntax is used but why is fork taken? #15

Open kaixin-hc opened 2 years ago

kaixin-hc commented 2 years ago

Screenshot 2021-11-12 at 5.38.10 PM.png

Confusing. Activity diagram should show what is done.

Why would list not be displayed? condition is not clear ("display list to change after command") sounds like the effect

nus-pe-bot commented 2 years ago

Team's Response

We agree that changing the phrasing of the condition would have been clearer. However, since it is just a phrasing issue, and the main concept is still being conveyed, we believe it is not justified to rate it Low severity. If display list to change after command is still a valid condition, and is not completely wrong. Since the activity diagram is still accurate, we have downgraded the severity.

Items for the Tester to Verify

:question: Issue severity

Team chose [severity.VeryLow] Originally [severity.Low]

Reason for disagreement: Clarifying my bug report -- I flagged this whole diagram under the "The diagram is too complicated." documentation bug category.

I think that the choices in this model overly complicate a simple process, and maybe aren't even using the UML diagram correctly. From the textbook:

"An action is a single step in an activity...

The "Return a new CommandResult..." is actually part of the execute() step, unless the returning of Command is a separate action? Similarly the control flow where the condition was [display list to change after command] is once again separating out two conditions which seems like it should be actually one condition from the phrasing. (From your response, I judge that you actually meant to show that "display list to change after command" as a separate boolean condition, which would be fine provided that the phrasing showed it was a condition and not a description of the coming actions).

I recognise that my comment that "Activity diagram should show what is done" could have been clearer. What I meant is that following the textbook:

activity diagrams (AD) can model workflows

However, the current diagram delves into coding implementation details("Returns a new CommandResult with the field ListDisplayChange.ELDERLY) instead of just showing the logical process of the workflow. That is not an activity diagram's purpose, and further implies that after the Main Window (since that is what is being summarised here, according to your explanation.) returns a new command result, it can somehow still clear the lists --> which contradicts what return is supposed to do.

The choices in this diagram do impede understanding, so I think it is still severity.Low