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

Error message for invalid Rate does not match actual behavior #3553

Open
nus-pe-bot opened this issue Apr 16, 2022 · 1 comment
Open

Error message for invalid Rate does not match actual behavior #3553

nus-pe-bot opened this issue Apr 16, 2022 · 1 comment

Comments

@nus-pe-bot
Copy link

nus-pe-bot commented Apr 16, 2022

Steps to reproduce:

  1. Ensure there's at least 1 person in the list
  2. Perform personedit 1 r/a (or any other invalid rate)

Expected: Error message should state that the rate must be a non-negative rational number

Actual: Error message states that "The rate should not be negative or have more than 2 decimal places". There is no mention that the rate must be a rational number. Furthermore, the error message is incorrect as rates with more than 2 decimal places are allowed (the program just rounds it off to 2 decimal places)
2022-04-16_14-36.png


[original: nus-cs2103-AY2122S2/pe-interim#3534] [original labels: severity.Low type.FunctionalityBug]
@zhongfu
Copy link
Contributor

zhongfu commented Apr 18, 2022

Team's Response

We do not think that this issue qualifies, as:

  • floating point numbers (as used in Java) are by definition rational, since they are limited in precision
  • we only expect a plain number, which means that users cannot enter reasonably enter an irrational number
  • the term "rational" may not be familiar to all users, and using this term will only serve to confuse them more

Additionally, while we do not specify that the input has to be a "number", we believe that this will be obvious enough to users, since "negative" and "2 decimal places" implies that the input should be a number of some sort. (A "negative word" or "word with 2 decimal places" doesn't make too much sense in the context of this command, after all.)

Duplicate status (if any):

--

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