VaishakAnand / pe

0 stars 0 forks source link

ViewTutorialGroupCommand Sequence Diagram involves too many low-level details #8

Open VaishakAnand opened 3 years ago

VaishakAnand commented 3 years ago

I believe that a lot of these low-level details can be abstracted away (no need to go deeper than UniqueModule List). This applies to the other sequence diagrams as well, but is especially apparent for this particular command.

image.png

nus-se-bot commented 3 years ago

Team's Response

Thanks for the suggestion. The lower levels of the sequence diagram were intentionally not abstracted out as it is neccessary information for the reader to know. We intended for it to explicitly show which classes are involved in the method execution.

However we accept this as a formatting error because it may be difficult for the reader to comprehend. Hence the VeryLow severity.

Items for the Tester to Verify

:question: Issue severity

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

Reason for disagreement: The team has conceded that the issue may cause difficulty for the reader to comprehend the sequence diagram, but isn't the whole point of the sequence diagram to help the users comprehend the inner workings of the application? According to the cs2103 website, a VeryLow rating is applicable to bugs that involve 'a typo/spacing/layout/color/font issues in the docs'.

I don't think that this issue is as simple as a 'formatting error', as there would be not logical way to address the issue (due to horizontal size limitations), other than either:

a) Splitting up the sequence diagram into separate diagrams, or b) deciding to abstract away lower level details that I truly believe should be done (e.g. is it really necessary to include the get(index) function call for a List in the sequence diagram)

As such, i believe that the only way to solve this issue is by reworking the entire sequence diagram, and so should not just be considered a minor 'formatting' issue. Therefore, I think that a Low rating is more appropriate.