sandyk0105 / pe

0 stars 0 forks source link

Invalid index 0 gives incorrect format error message #1

Open sandyk0105 opened 2 weeks ago

sandyk0105 commented 2 weeks ago

The command delete 0 should not give an incorrect command format error message, but an invalid input error message instead. image.png

You also mentioned that 0 is an invalid index in the UG.

image.png

If you want to be consistent, you should provide why 0 is an invalid input just like using (+1234) for phone country code

image.png

nus-se-script commented 1 week ago

Team's Response

To decide whether this is a valid feature flaw, we consult the course page: image.png

The error message given supplies enough info to diagnose the mistake. From your screenshot:

image.png

i.e the INDEX (must be a positive integer), which is specific information needed to resolve the issue. For an index of 0, this does fall under incorrect format, as the format of the command itself required the index to be positive.

This assumes the user understands what a positive integer is. We made this assumption because financial consultants are likely to understand this, so there is no need for a more "specified" message like "0 is not a positive integer!", which would have offered no extra benefit to these math-literate financial consultants.

Furthermore: image.png

But note that other error messages also do not directly specify the errors beyond the input being wrong. From your example, both phone numbers (+1234) 9999 and 9999 [note that is too long] would've shown the same error message.

As a gesture of good faith, perhaps you wanted a more robust, customised system of error messaging? So delete -1 would yield "-1 is a negative number and not a positive integer", or delete 0 would yield "0 is a zero integer, and not a positive integer"? That level of specificity is nice to have, but ultimately out of scope because we delivered other more substantive features. We still want to give some credit for the tester, hence our response.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: [replace this with your explanation]