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

Fix bug when max scores were negative, and feedback given #111

Merged
merged 3 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/main/java/seedu/address/commons/util/StringUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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} <br>
* 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;
}
Comment on lines +79 to +85

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.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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] "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

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

}

AddAssignmentCommand.AssignmentDescriptor assignmentDescriptor =
new AddAssignmentCommand.AssignmentDescriptor(maxScore, assignmentName);

return new AddAssignmentCommand(studentIndex, assignmentDescriptor);
}
}
31 changes: 11 additions & 20 deletions src/main/java/seedu/address/logic/parser/ParserUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -118,13 +119,10 @@
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);
Comment on lines -121 to +125

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.

}
/**
* Parses a {@code String score} into a {@code int}.
Expand All @@ -135,30 +133,23 @@
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}.
* Leading and trailing whitespaces will be trimmed.
*
* @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();

Check warning on line 148 in src/main/java/seedu/address/logic/parser/ParserUtil.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/seedu/address/logic/parser/ParserUtil.java#L148

Added line #L148 was not covered by tests
if (!StringUtil.isNonZeroUnsignedInteger(trimmedIndex)) {
throw new ParseException(MESSAGE_INVALID_INDEX);

Check warning on line 150 in src/main/java/seedu/address/logic/parser/ParserUtil.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/seedu/address/logic/parser/ParserUtil.java#L150

Added line #L150 was not covered by tests
}
return parsedStudentIndex;
return Integer.parseInt(trimmedIndex);

Check warning on line 152 in src/main/java/seedu/address/logic/parser/ParserUtil.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/seedu/address/logic/parser/ParserUtil.java#L152

Added line #L152 was not covered by tests
}
/**
* Parses {@code Collection<String> tags} into a {@code Set<Tag>}.
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/seedu/address/model/assignment/Assignment.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
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);

Check warning on line 61 in src/main/java/seedu/address/storage/JsonAdaptedAssignment.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/seedu/address/storage/JsonAdaptedAssignment.java#L61

Added line #L61 was not covered by tests
}

final AssignmentName modelAssignmentName = new AssignmentName(assignmentName);
Expand Down