search

nzixuan / pe

0 stars 0 forks source link

Omission of AddressBookParser in sequence diagram #8

Open nzixuan opened 2 years ago

nzixuan commented 2 years ago

AddressBookParser class is omitted from the diagram implying that LogicManager is calling the parserCommand in TaskCommandParser directly instead of through the AddressBookParser

Screenshot 2021-11-12 at 5.28.36 PM.png

nus-pe-bot commented 2 years ago

Team's Response

Screenshot 2021-11-14 at 11.40.45 PM.png

The reason why I decided to remove the AddressBookParser from the sequence diagram is to simplify the diagram. However, to avoid confusion I have explained about the AddressBookParser in my accompanying written paragraph and there is also a note at the end to state that the AddressBookParser was omitted for simplicity of the diagram. According to the screenshot above (taken from the website), this is considered to be okay and not a bug.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: This representation of the sequence diagram implies that parseCommand("task -e 3 n/Report 4") from TaskCommandsParser is called when actually parseCommand("-e 3 n/Report 4") from TaskCommandsParser is called which could mislead the reader of the diagram on what arguments are passed into TaskCommandsParser the paragraph include does not specifically mention anything regarding this. Perhaps using parseCommand("-e 3 n/Report 4") in the diagram would be a better representation of the code.