brelkh / pe

0 stars 0 forks source link

Adding property to buyer gives different error for index out of bounds #14

Open brelkh opened 2 years ago

brelkh commented 2 years ago

image.png

image.png

Different errors are given for 0 and index more than list length. This can be confusing for the user when they read invalid command format for index 0 input.

nus-pe-script commented 2 years ago

Team's Response

This is a similar issue we received, regarding our english phrase "invalid command format".

The 'Original' Bug

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

Wrong error displayed for index out of bounds (delete index 0)

image.png

Error should be the following, rather than "incorrect command format":

image.png


[original: nus-cs2103-AY2122S2/pe-interim#1076] [original labels: severity.Low type.FeatureFlaw]

Their Response to the 'Original' Bug

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

This is a cosmetic issue. We did address it in the QNA section:

image.png.

This is because by design, we check whether the parsed number is a positive integer first, before we check the size. In our error message shown, we still state that our index must be a positive integer for both the commands.

Perhaps we can improve on the English of the phrase "invalid command format"

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: [replace this with your explanation]


:question: Issue severity

Team chose [severity.VeryLow] Originally [severity.Low]

Reason for disagreement: I agree that this is a duplicate since updating the ParserUtil could fix problems that are duplicates of this. However, similar to the duplicate, I do not agree with the team calling this a cosmetic issue.

The following explanation is the same as what I explained in the duplicate:

Firstly, the group agrees that this is a bug by not rejecting this bug.

However, the group says that this is a cosmetic issue. According to PE instructions:

image.png

I disagree with the group's opinion of this being purely cosmetic, since this issue is not a "typo/spacing/layout/color/font issue". As the group highlights, "improving on the English" suggests it is not merely a typo, but a failure to communicate their idea with the reader appropriately. Furthermore, this issue does affect usage as the user will be confused about what is causing their input to be rejected due to the inconsistency.

As such, the severity cannot be veryLow, which is reserved for purely cosmetic flaws.