yutingzou / pe

0 stars 0 forks source link

Wrong method called in DG 4.8 Delete multiple tasks with an index range, sequence diagram #11

Open yutingzou opened 3 years ago

yutingzou commented 3 years ago

It is not guaranteed that a matched task is found so generateSuccessMessage() is not always called.

Screenshot 2021-04-16 at 3.31.31 PM.png

nus-pe-bot commented 3 years ago

Team's Response

While it is unclear from the diagram, readers might think getTasksToDelete will throw an exception where there are no matching tasks, which is actually our implementation. While it can be argued that the exception throwing could be shown in the sequence diagram above, it can also be argued that excluding it has its benefits because it would include too many additional details into the sequence diagram that might not be very important. After all, if we wanted to show the exact execution flow of deleting multiple tasks using an index range, we would have used an activity diagram instead.

If we were to explicitly state all the possible exceptions thrown from this execution, there would be much more to include (e.g DeleteMultipleCommandParser#parse and ParserUtil#parseMultipleIndex can throw exceptions when parsing) , and the cluttering of the sequence diagram with less important details would be less appropriate than one without these details. The only issue we find with this diagram is that we did not complement it with an activity diagram or use an activity diagram instead. We are arguing for low severity instead, since it is a "minor inconvenience" rather than an "occasional inconvenience" that we used a sequence diagram to explain how this function of Taskify is executed rather than an activity diagram.

Items for the Tester to Verify

:question: Issue severity

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

Reason for disagreement: [replace this with your reason]