search

casperngeen / pe

0 stars 0 forks source link

Valid ID input but error message is shown #1

Open casperngeen opened 2 weeks ago

casperngeen commented 2 weeks ago

Steps to reproduce:

  1. Enter an extremely long number as an ID when using add commands

Expected: Contact can be added or integer overflow error message Actual: ID should only contain numeric characters

image.png

nus-pe-bot commented 1 week ago

Team's Response

Thanks for the bug report. It is true that the current error message is bad and may be confusing to the user.

image.png

However, as mentioned in our DG Planned Enhancements section (see https://ay2425s1-cs2103t-t15-4.github.io/tp/DeveloperGuide.html#planned-enhancements), we plan to fix this by providing a better error message in future updates.

Furthermore, we think that this can be considered as extreme user behavior:

image.png

Thus, we are not considering this a bug for version 1.6.

We think that this is a duplicate of #650 because both issues can be resolved by correcting the input validation of the Id class.

The 'Original' Bug

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

Add command does not work with integers that are too large

Steps to Reproduce

  1. Type the command add id/2147483648 n/John Doe p/98765432 e/johnd@example.com a/John street, block 123,#01-01

Expected Behavior: Adds a contact named John Doe to the Address Book.

Observed Behavior: An error message appears, stating that the ID should only contain numeric characters. However, we can see that the ID only contains the digits from 0-9. This was likely due to the developing team using an Integer to store the ID, which can only store a maximum value of 2147483647 in Java.

Bug 1.png

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

Their Response to the 'Original' Bug

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

Hi, thanks for pointing this out. However, this is a known issue mentioned under "Planned Enhancements" in our Developer Guide (see https://ay2425s1-cs2103t-t15-4.github.io/tp/DeveloperGuide.html#planned-enhancements).

image.png

Furthermore, we think that the severity of this "FeatureFlaw" should be "Low", because it is a flaw "that is unlikely to affect normal operations of the product" and "appears only in very rare situations and causes a minor inconvenience only". This is because it is rather rare for an employee id to exceed 2 billion. For example, Walmart only has around 2 million employees. Therefore, it is quite likely that the user can simply choose another unused id less than 2 billion as a workaround.

Nevertheless, thanks for raising this issue.

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 response Team chose [`response.Rejected`] - [x] I disagree **Reason for disagreement:** I believe this should not be considered an enhancement, as the error message right now is inaccurate, and not just unclear or ambiguous. This is misleading for the users, therefore it would still cause an inconvenience to the user when they read the error message.
## :question: Issue type Team chose [`type.FeatureFlaw`] Originally [`type.FunctionalityBug`] - [x] I disagree **Reason for disagreement:** I do not agree that this is a Feature Flaw as the issue here is the inaccurate error message, instead of the product design being flawed.