Inconsistent check command error message for invalid indices

[leslieyip02]

Description

When using an invalid index with the check command, the error messages are not consistent. When using an index that is greater than the size of the list of clients, the error message is The Client index provided is invalid.. When using indices such as 0 or -1, the error message is Invalid command format!, which is less precise.

Steps to Reproduce

1. check 0

Expected Result

[nus-se-bot]

Team's Response

No details provided by team.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

Inconsistent view command error message for invalid indices

Description

When using an invalid index with the view command, the error messages are not consistent. When using an index that is greater than the size of the list of clients, the error message is The Client index provided is invalid.. When using indices such as 0 or -1, the error message is Invalid command format!, which is less precise.

Steps to Reproduce

1. view 0

Expected Result

---

_{[original: nus-cs2103-AY2425S1/pe-interim#784] [original labels: severity.Low type.FunctionalityBug]}

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

Thank you for your feedback. However, this issue does not constitute a bug. Below is the rationale for rejecting this report:

Reason for duplication

The command's input validation for indices parsed when executing view, check, and del-client are handled by the same method ParserUtil:parseIndex(). In order for input validation handled by individual Commands ViewClientCommand, CheckClientCommand and DeleteClientCommand, we can only "fix the defect" by changing the parseIndex method to not throw the ParseException. However, our team still firmly believes that it is a better practice to handle such inputs (zero, negative) at the parseIndex function and do not deem this as a defect, as it reduces unnecessary code repetition, as well as segregates issues caused by command format (e.g. zero/ negative integers) vs issues caused by other factors (e.g. client index does not exist in the list). In this case, as the index does not follow the command specifications (must be positive integer), we deem such inputs as Invalid command format!. (more below)

Reason for rejection (Please see Equivalence Partitions for more information)

NOTE: the context used here is view, but it applies to both check and del-client as well (see reason for duplication).

As stated in the screenshots for view 0 and view -1 and the UG, Parameters: INDEX (must be a positive integer), MATER thus behaved as expected, alerting the user that the format of the command is invalid due to an input of zero/ negative integer.

In the case of view 8, MATER behaved as expected. The command format is correct as 8 is a positive integer. MATER then behaved as expected, correctly alerts the user that the index 8 provided is invalid as the client does not exist on the list.

Equivalence Partitions

1. [MIN_INT ... 0]: Invalid command format! due to input of zero/ negative integer, which goes against command specifications (must be a positive integer). Rejected by Parser as it is an issue with command format.
2. [1 ... N]: (where N = number of clients in the list) Successfully view client details.
3. [N + 1 ... MAX_INT]: The Client index provided is invalid. as a client with the index does not exist on the list. However, the command format is still correct as it abides by the command specifications (must be a positive integer). Rejected by Command as it is an issue caused by factors other than command format.

Screenshot of Error Message.

Error Message clearly states the command specifications, where integer must be positive.

Screenshot of UG.

UG clearly states the command specifications, where integer must be positive.

Items for the Tester to Verify

:question: Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

• [ ] I disagree

Reason for disagreement: [replace this with your explanation]

---

## :question: Issue response Team chose [`response.Rejected`] - [x] I disagree **Reason for disagreement:** Same reasoning as the original. ## Actual Bug `ParserUtil::parseIndex` throws a `ParseException` with `MESSAGE_INVALID_INDEX`, but this message is not actually shown to the user. The message shown to the user should be `"Index is not a non-zero unsigned integer."` instead of `"Invalid command format!"`.  I think the problem lies here in `CheckClientCommandParser` because the message of the thrown `pe` is not actually used.  `pe` is supplied as the `cause` argument of `ParseException`, so `MESSAGE_INVALID_INDEX` remains unused in the error message to the user. 

---