editPerson and editBook do not check for invalid index

[gremmyz]

Type: Design Flaw

Currently,

there is no check for a person out of the valid person index before checking for valid arguments. Perhaps a check could be done here so that users know that the person index is out of bounds.

Edit: Tested deletePerson 24

Perhaps the editPerson 24 return error message could be something like this as well.

Edit 2: same goes for editBook

[soc-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]

Consistency of command error messages

Type: Design Flaw

Currently,

addPerson returns the command feedback message:

However, editPerson and deletePerson return these respectively:

Perhaps it would be good if these error feedback messages were more consistent, i.e. if editPerson and deletePerson both return the usage of the command and an example command in their error message.

Similarly, for editPerson

perhaps the fields that are editable can be shown in the error message here, as it can perhaps cause minor inconveniences to new users who are trying out the command.

---

_{[original: nus-cs2103-AY2223S2/pe-interim#3886] [original labels: severity.VeryLow type.FeatureFlaw]}

Their Response to the 'Original' Bug

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

Ok good catch the error messages could be more consistent and clear. Will add in the future.

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)

• [x] I disagree

Reason for disagreement: I don't think this is a duplicate.

Referring to your code for EditPersonCommandParser,

throw new ParseException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX, pe);

it is this section that results in this error message:

and should be something like this instead:

throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditCommand.MESSAGE_USAGE), pe);

This will resolve the 'Original' issue.

The other issue is when a PERSON_INDEX greater than the number of people in the person list (and book list too, for editBook), it does not check that it is a wrong index.

It's due to a missing validity check around line 45 here:

where there should be a check to see if the input index is greater than the number of Person (or Book, for editBook) objects in the list, something like this perhaps:

In my opinion, since these are two distinct locations and this buggy behaviour can be fixed independently, they should not be considered as duplicates.

---