UdhayaShan1 / pe

0 stars 0 forks source link

Unclear negative `INDEX` errors #10

Open UdhayaShan1 opened 7 months ago

UdhayaShan1 commented 7 months ago

edit -1 n/wdad as expected is a negative index. But it simply returns

image.png

Yes while the INDEX (must be a positive integer) is there, this is not the focus of the error message. It prints invalid command first and the correct format indicating a generic invalid message.

Error message should be more specific.

nus-pe-script commented 7 months ago

Team's Response

Refer to #1356. This is just referring to error messages.

The 'Original' Bug

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

Inconsistent out of bounds INDEX error

image.png

For list of 9 people,

edit 21321312131232132132132132131313123 n/wdad returns

image.png

and

edit 2323 n/wdad returns

image.png

It should be the same out of bounds expected error.

This is separate from negative index issue as they are not fixed the same way.


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

Their Response to the 'Original' Bug

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

Hi thanks for the catch!

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: I wished the team would provide better justification as to why this was a duplicate of another issue I flagged where I mentioned below "This is separate from negative index issue as they are not fixed the same way." to bring in some possible discussion with the dev team.

To why I believe this was not an issue,

All the team mentioned was "Refer to #1356. This is just referring to error messages.". This is too generic and not acknowledging if they understood the difference.

Both are separate issues that are fixed by separate code changes


## :question: Issue response Team chose [`response.NotInScope`] - [x] I disagree **Reason for disagreement:** I do not think this is NotInScope, NotInScope entails that the UG mentions this to justify why possible effort spent on this is not worthwhile compared to other features. All was mentioned "Refer to #1356. This is just referring to error messages.", this is just referring to error messages which in this case is a generic error message FeatureFlaw. ![image.png](https://raw.githubusercontent.com/UdhayaShan1/pe/main/files/2b3e893c-86a1-4410-8bf9-3e981542f900.png) Since, the team did not offer **justification in UG** for why they have generic commands for negative index or for **effort considerations**, I disagree.