search

benedictkhoomw / pe

0 stars 0 forks source link

DG: Review Mode Sequence Diagram another unexplained return arrow #11

Open benedictkhoomw opened 3 years ago

benedictkhoomw commented 3 years ago

Where it is

DG -> [Implemented] Review feature -> the only sequence diagram in this sub-section

Issue and justification

There is no corresponding call from the User to ReviewMode, but there is an unexplained return arrow. This is going to confuse most readers who will be wondering what it represents.

Evidence

image.png

nus-pe-bot commented 3 years ago

Team's Response

The returned arrow represents the output that is displayed to the user. The input is the user input in the CommandBox, whether it is n, p, a, h,..., it will have different output displayed to the user in the ResultDisplay. I think the only thing that is unclear is that I need to label that arrow, so that the readers know that is the feedback to the user when he/she enters a command in Review Mode. Since this flaw only causes some inconvenience to the readers, Medium severity is more suitable for this bug.

Items for the Tester to Verify

:question: Issue severity

Team chose [severity.Medium] Originally [severity.High]

Reason for disagreement: The return arrow there is saying "the function returns control to the caller" which makes no sense because it was never called. Note that ReviewMode was constructed and the constructor returned (ReviewMode is no longer active). After that, there was no further call to ReviewMode to activate it. And yet it still continues to do a lot of stuff, including return two more times to 'callers' that never called it.

It is not a simple matter of just labeling the return arrow; the diagram would still be incorrect and confusing to many readers familiar with UML but not familiar with the codebase (i.e. the target audience of the DG). A reader familiar with UML notation will be confused by the diagram because it is incorrect (once a diagram is incorrect, the reader has to guess what the author is trying to say; and the purpose of a diagram is to clarify, not make uncertain). Therefore, the severity is high, not medium.

For more details on why it is a notation error, see the diagram below or check out the relevant section of the textbook.

image.png