JulietTeoh / pe

0 stars 0 forks source link

Invalid index message for Edit command is inconsistent #3

Open JulietTeoh opened 3 years ago

JulietTeoh commented 3 years ago

Screenshot 2021-04-16 at 2.23.40 PM.png Screenshot 2021-04-16 at 2.23.58 PM.png Screenshot 2021-04-16 at 2.24.10 PM.png Screenshot 2021-04-16 at 2.24.21 PM.png

These commands should all provided the same error message as its index is invalid: edit property 100 n/Bishan p/570150

edit property 0 n/Bishan p/570150

edit property -1 n/Bishan p/570150

edit property 100000000000000 n/Bishan p/570150

However, they provide 2 different error messages, which can be misleading to users.

Placing this as severity.Low as it can confuse users despite having the same type of error.

nus-pe-bot commented 3 years ago

Team's Response

No response provided.

The 'Original' Bug

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

For entering invalid index for update, different error messages given

When the index is less than 0, one error message and with index = 0 or more than elements in list, error message highlighting index is shown. bug10.png


[original: nus-cs2103-AY2021S2/pe-interim#2932] [original labels: severity.Medium type.FeatureFlaw]

Their Response to the 'Original' Bug

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

There was a discussion regarding how negative values should be handled. While trying to fix a bug found during PE dry-run, we decided to show an invalid command error for negative values as we believed that the average user would not perceive the "-" in front of the INDEX as a negative INDEX but as an invalid command. As a result, more code was added to ensure that "-" in front of the INDEX would show the invalid command error. Furthermore, the UG mentioned that only positive integers should be used.

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)

Reason for disagreement: Your instructions for manual testing did not clearly state how index errors would be handled. Thus, users would think that these commands should all provided the same error message as the error here is that index:

edit property 100 n/Bishan p/570150 edit property 0 n/Bishan p/570150 edit property -1 n/Bishan p/570150 edit property 100000000000000 n/Bishan p/570150

By having different error messages, it can be confusing to the user, especially when 100 and 100000000000000 are both numbers larger than the size of the list. Thus, this bug still holds.


:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: [replace this with your explanation]