-
Notifications
You must be signed in to change notification settings - Fork 5
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
Fix bug when max scores were negative, and feedback given #111
Fix bug when max scores were negative, and feedback given #111
Conversation
Negative max scores caused a bug As a side-effect of the fix, error messages may not be as specific, specifically, entering a negative score for the grade command
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good!
try { | ||
int value = Integer.parseInt(s); | ||
// "-0" and "+0" is successfully parsed by Integer#parseInt(String) | ||
return value >= 0 && !s.startsWith("+") && !s.startsWith("-"); | ||
} catch (NumberFormatException nfe) { | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job on ensuring the input is a valid unsigned integer and not a signed one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, | ||
AddAssignmentCommand.MESSAGE_USAGE), pe); | ||
throw new ParseException( | ||
String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddAssignmentCommand.MESSAGE_USAGE), pe); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good formatting to follow coding standard
int parsedScore; | ||
try { | ||
parsedScore = Integer.parseInt(trimmedMaxScore); | ||
} catch (NumberFormatException e) { | ||
throw new ParseException("The score must be an integer!"); | ||
if (!StringUtil.isUnsignedInteger(trimmedMaxScore)) { | ||
throw new ParseException(Assignment.MAX_SCORE_MESSAGE_CONSTRAINTS); | ||
} | ||
return parsedScore; | ||
return Integer.parseInt(trimmedMaxScore); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a similar issue happening in gradeAssignmentCommand. Great attempt to simplify the code.
Negative max scores caused a bug
As a side-effect of the fix, error messages may not be as specific, specifically, entering a negative score for the grade command