Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No error thrown for single-digit INDEX with prefix "0" for delete #4

Open
Javiery3889 opened this issue Apr 19, 2024 · 1 comment
Open

Comments

@Javiery3889
Copy link
Owner

Javiery3889 commented Apr 19, 2024

Describe the bug

The UG currently states the following for the delete command:

Screenshot 2024-04-19 at 16.58.49.png

If the user enters a single digit INDEX with a prefix string of "0" prepended, the command still runs successfully.

Steps to reproduce

Assume that the list has at least one person
delete 0000000001

Expected

Throw an error for invalid INDEX

Actual

Person at index 1 is deleted successfully

Suggested fix

Perhaps it would be better to reject such instances and throw an error for invalid index instead

@nus-pe-script
Copy link

nus-pe-script commented Apr 22, 2024

Team's Response

Thank you for the issue.

By intuition, 1 equals 000001 and so on. Hence inputting “0000001”, or any number of leading “0”s does not change the fact that it is still an integer, and its value is still 1.

Hence as long as there is a valid contact at index 1, the app will treat the input as valid and correctly delete the contact.

Items for the Tester to Verify

❓ Issue response

Team chose [response.Rejected]

  • I disagree

Reason for disagreement: Thank you for your response!

While I do agree that the intuition of having an INDEX appended with a string of 0 will be treated the same, I felt that this behaviour was not highlighted in the UG and might cause some users to be unaware of this beaviour. Perhaps it would be better to explicitly state this behaviour in the UG, similar to the behaviour of extraneous parameters for commands which was documented in the UG.

Screenshot 2024-04-23 at 10.48.43.png

Because of this, I would categorise this as type.DocumentationBug instead with the same severity of severity.Low.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants