pzl111 / pe

0 stars 0 forks source link

"delete 0" should mention wrong index, not wrong command format #21

Open pzl111 opened 10 months ago

pzl111 commented 10 months ago

image.png

As mentioned in CS2103T textbook, "Error messages can be correct but not specific enough" can be counted as feature flaws. It should say wrong index, not wrong format, since 0 is technically a number, and the command is in the correct format, just that it has a wrong index. The app should mention the incorrect index rather than incorrect format. However I put this as low severity since it is easy to debug what the problem is, but not very low severity because it is wrong information and not a cosmetic issue.

image.png

nus-se-script commented 9 months ago

Team's Response

Thank you for your feedback! The error messages are actually very specific, as our intention was to separate the cases for the user, and to give them specific error messages as they are separate flows:

Invalid Command Format: Inputting an index that will never be valid. We specify in this error that the index must be a positive integer, i.e. negative numbers and 0 are rejected.

Invalid Transaction Index: Inputting an index that is invalid because it is out of bounds (e.g. list is of size 8, input index is 10), not because the integer is invalid. We specify in this error that the transaction index is invalid to inform the user that it doesn't exist in the current context.

We also specify this in our UG, under Argument Types, explaining why this is part of our command format:

image.png

We will thus be rejecting this as a bug.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: [replace this with your explanation]