From a705488b47f882a17b5cf071a320c7a1027da930 Mon Sep 17 00:00:00 2001 From: lm-44 Date: Tue, 22 Oct 2024 00:04:12 +0800 Subject: [PATCH] Fix bug when max scores were negative, and feedback given 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 --- .../address/commons/util/StringUtil.java | 19 ++++++++++++ .../commands/GradeAssignmentCommand.java | 2 +- .../parser/AddAssignmentCommandParser.java | 10 +++--- .../address/logic/parser/ParserUtil.java | 31 +++++++------------ .../address/model/assignment/Assignment.java | 9 ++++-- .../storage/JsonAdaptedAssignment.java | 4 +-- 6 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/main/java/seedu/address/commons/util/StringUtil.java b/src/main/java/seedu/address/commons/util/StringUtil.java index 61cc8c9a1cb..73cd5558624 100644 --- a/src/main/java/seedu/address/commons/util/StringUtil.java +++ b/src/main/java/seedu/address/commons/util/StringUtil.java @@ -65,4 +65,23 @@ public static boolean isNonZeroUnsignedInteger(String s) { return false; } } + + /** + * Returns true if {@code s} represents an unsigned integer + * e.g. 0, 1, 2, ..., {@code Integer.MAX_VALUE}
+ * Will return false for any other non-null string input + * e.g. empty string, "-1", "+1", and " 2 " (untrimmed), "3 0" (contains whitespace), "1 a" (contains letters) + * @throws NullPointerException if {@code s} is null. + */ + public static boolean isUnsignedInteger(String s) { + requireNonNull(s); + + 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; + } + } } diff --git a/src/main/java/seedu/address/logic/commands/GradeAssignmentCommand.java b/src/main/java/seedu/address/logic/commands/GradeAssignmentCommand.java index 47b087aa2b1..bba23c76bdd 100644 --- a/src/main/java/seedu/address/logic/commands/GradeAssignmentCommand.java +++ b/src/main/java/seedu/address/logic/commands/GradeAssignmentCommand.java @@ -24,7 +24,7 @@ public class GradeAssignmentCommand extends Command { public static final String MESSAGE_USAGE = COMMAND_WORD + ": Edits the score of the assignment belonging to a " + "student by the student's index number used in the displayed student list, and the assignment's index" - + " belonging to the student." + + " belonging to the student. " + "The assignment will also be marked as Submitted.\n" + "Parameters: " + "[" + PREFIX_STUDENT_INDEX + "INDEX] " diff --git a/src/main/java/seedu/address/logic/parser/AddAssignmentCommandParser.java b/src/main/java/seedu/address/logic/parser/AddAssignmentCommandParser.java index 7867908c14f..1543b9269db 100644 --- a/src/main/java/seedu/address/logic/parser/AddAssignmentCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/AddAssignmentCommandParser.java @@ -30,24 +30,24 @@ public AddAssignmentCommand parse(String args) throws ParseException { throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddAssignmentCommand.MESSAGE_USAGE)); } - argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_STUDENT_INDEX, PREFIX_ASSIGNMENT_NAME, - PREFIX_ASSIGNMENT_MAX_SCORE); - Index studentIndex; AssignmentName assignmentName; int maxScore; + argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_STUDENT_INDEX, PREFIX_ASSIGNMENT_NAME, + PREFIX_ASSIGNMENT_MAX_SCORE); try { studentIndex = ParserUtil.parseIndex(argMultimap.getValue(PREFIX_STUDENT_INDEX).get()); assignmentName = ParserUtil.parseAssignmentName(argMultimap.getValue(PREFIX_ASSIGNMENT_NAME).get()); maxScore = ParserUtil.parseMaxScore(argMultimap.getValue(PREFIX_ASSIGNMENT_MAX_SCORE).get()); } catch (ParseException pe) { - 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); } AddAssignmentCommand.AssignmentDescriptor assignmentDescriptor = new AddAssignmentCommand.AssignmentDescriptor(maxScore, assignmentName); + return new AddAssignmentCommand(studentIndex, assignmentDescriptor); } } diff --git a/src/main/java/seedu/address/logic/parser/ParserUtil.java b/src/main/java/seedu/address/logic/parser/ParserUtil.java index 19af243b541..3f060560deb 100644 --- a/src/main/java/seedu/address/logic/parser/ParserUtil.java +++ b/src/main/java/seedu/address/logic/parser/ParserUtil.java @@ -10,6 +10,7 @@ import seedu.address.commons.core.index.Index; import seedu.address.commons.util.StringUtil; import seedu.address.logic.parser.exceptions.ParseException; +import seedu.address.model.assignment.Assignment; import seedu.address.model.assignment.AssignmentName; import seedu.address.model.student.Email; import seedu.address.model.student.Name; @@ -118,13 +119,10 @@ public static AssignmentName parseAssignmentName(String assignmentName) throws P public static int parseMaxScore(String maxScore) throws ParseException { requireNonNull(maxScore); String trimmedMaxScore = maxScore.trim(); - 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); } /** * Parses a {@code String score} into a {@code int}. @@ -135,13 +133,10 @@ public static int parseMaxScore(String maxScore) throws ParseException { public static int parseScore(String score) throws ParseException { requireNonNull(score); String trimmedScore = score.trim(); - int parsedScore; - try { - parsedScore = Integer.parseInt(trimmedScore); - } catch (NumberFormatException e) { + if (!StringUtil.isUnsignedInteger(trimmedScore)) { throw new ParseException("The score must be an integer!"); } - return parsedScore; + return Integer.parseInt(trimmedScore); } /** * Parses a {@code String studentIndex} into a {@code int}. @@ -149,16 +144,12 @@ public static int parseScore(String score) throws ParseException { * * @throws ParseException if the given {@code studentIndex} is invalid. */ - public static int parseStudentIndex(String studentIndex) throws ParseException { - requireNonNull(studentIndex); - String trimmedStudentIndex = studentIndex.trim(); - int parsedStudentIndex; - try { - parsedStudentIndex = Integer.parseInt(trimmedStudentIndex); - } catch (NumberFormatException e) { - throw new ParseException("The score must be an integer!"); + public static int parseStudentIndex(String oneBasedIndex) throws ParseException { + String trimmedIndex = oneBasedIndex.trim(); + if (!StringUtil.isNonZeroUnsignedInteger(trimmedIndex)) { + throw new ParseException(MESSAGE_INVALID_INDEX); } - return parsedStudentIndex; + return Integer.parseInt(trimmedIndex); } /** * Parses {@code Collection tags} into a {@code Set}. diff --git a/src/main/java/seedu/address/model/assignment/Assignment.java b/src/main/java/seedu/address/model/assignment/Assignment.java index af23254253a..eb7dc46b646 100644 --- a/src/main/java/seedu/address/model/assignment/Assignment.java +++ b/src/main/java/seedu/address/model/assignment/Assignment.java @@ -15,8 +15,11 @@ */ public class Assignment { - public static final String MESSAGE_CONSTRAINTS = + public static final String MAX_SCORE_MESSAGE_CONSTRAINTS = "The maximum score should be greater than or equal to the minimum score of 0"; + public static final String SCORE_MESSAGE_CONSTRAINTS = + "The score should be between (inclusive) of the minimum and maximum score"; + private static final int MIN_SCORE = 0; // Identity fields @@ -36,14 +39,14 @@ public Assignment(Student student, AssignmentName name, int maxScore) { requireNonNull(name); this.student = student; this.assignmentName = name; - checkArgument(isValidScore(maxScore), MESSAGE_CONSTRAINTS); + checkArgument(isValidMaxScore(maxScore), MAX_SCORE_MESSAGE_CONSTRAINTS); this.maxScore = maxScore; } /** * Returns true if a given maxScore is a valid score. */ - public static boolean isValidScore(int test) { + public static boolean isValidMaxScore(int test) { return test >= MIN_SCORE; } diff --git a/src/main/java/seedu/address/storage/JsonAdaptedAssignment.java b/src/main/java/seedu/address/storage/JsonAdaptedAssignment.java index e2e404742f1..c164df06d29 100644 --- a/src/main/java/seedu/address/storage/JsonAdaptedAssignment.java +++ b/src/main/java/seedu/address/storage/JsonAdaptedAssignment.java @@ -57,8 +57,8 @@ public Assignment toModelType(Student student) throws IllegalValueException { if (!AssignmentName.isValidName(assignmentName)) { // Assuming you have this validation throw new IllegalValueException("Invalid Assignment Name"); } - if (!Assignment.isValidScore(maxScore)) { - throw new IllegalValueException(Assignment.MESSAGE_CONSTRAINTS); + if (!Assignment.isValidMaxScore(maxScore)) { + throw new IllegalValueException(Assignment.MAX_SCORE_MESSAGE_CONSTRAINTS); } final AssignmentName modelAssignmentName = new AssignmentName(assignmentName);