tianyue58 / pe

0 stars 0 forks source link

Execute() method in Sequence diagram needs to take in “model” as parameter #14

Open tianyue58 opened 2 years ago

tianyue58 commented 2 years ago

The parameter is absent in all the related uml where other developers may interpret this method as a method that does not take in any parameter, but in your code, it takes in model.

Screenshot 2021-11-12 at 5.35.24 PM.png

nus-pe-bot commented 2 years ago

Team's Response

A sequence diagram captures the interactions between multiple objects for a given scenario (in this case, to view a student).

The parameters were omitted to avoid low level details of the execute method, and to treat the method as a 'black box'. The absence of parameters for the execute method is unlikely to affect normal operations of the product, and it appears only in very rare situations and causes a minor inconvenience for the reader.

I accept that it may be unclear and may cause slight confusion to the reader and hence it is Low and not Very Low.

Items for the Tester to Verify

:question: Issue severity

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

Reason for disagreement: I disagree with the downgrading of severity for the following reasons.

First of all, there are two execute methods in the UML diagram. One of them takes in String and one of them takes in Model. I believe it is important to distinguish both by clearly showing the different argument type. Secondly, the execute() looks like a method which does not take in any argument which does not match the code. This could be particularly confusing for readers when they try to get familiarised to the code base for the first time and trace the code using IDE and compare their results with the UML diagrams. Lastly, this mistake occurs in almost all the UML diagrams in DG. As it is indeed confusing for readers and has repeated occurrence, I think the level of severity should be medium.

Hope this explanation helps. Thank you!