nus-se-bot
commented
1 year ago Team's Response
Accepted but this is a cosmetic issue and does not hinder the reader's ability to understand the class diagram.
Items for the Tester to Verify
:question: Issue severity
Team chose [severity.VeryLow]
Originally [severity.Low]
- [x] I disagree
Reason for disagreement: I disagree that this is a cosmetic issue because there is nothing cosmetic about this.
This diagram shows me that developers dumped as much information as they could without considering the perspective of the reader. Furthermore, the steps they listed did not even mention the method names that were listed in the class diagram.
I also mentioned in the original issue that it take longer to understand what that class is suppose to do due to a lot of things in it.
In conclusion, I disagree with the severity the developers chose and firmly believe that this should be a Low severity instead.
For the Parser -> Input, CommandChecker
For the ModuleList
As seen in the images above, there are too many things listed in the class and it hinders my reading. Perhaps only including the relevant ones based on what you are going to explain is good.
Justification: This is listed as Low severity as I can still read it, but I just need to take longer to understand what that class is suppose to do due to a lot of things in it.