Skip to content
This repository has been archived by the owner on Apr 6, 2024. It is now read-only.

Fix duplicate tag bug, error message specificity issue and standardize messages for notes feature #208

Merged
merged 6 commits into from
Nov 11, 2023
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
11 changes: 4 additions & 7 deletions src/main/java/seedu/address/commons/util/StringUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,12 @@ public static String getDetails(Throwable t) {
* Will return false for any other non-null string input
* e.g. empty string, "-1", "0", "+1", and " 2 " (untrimmed), "3 0" (contains whitespace), "1 a" (contains letters)
* @throws NullPointerException if {@code s} is null.
* @throws NumberFormatException if {@code s} is not a number.
*/
public static boolean isNonZeroUnsignedInteger(String s) {
public static boolean isNonZeroUnsignedInteger(String s) throws NumberFormatException {
requireNonNull(s);

try {
int value = Integer.parseInt(s);
return value > 0 && !s.startsWith("+"); // "+1" is successfully parsed by Integer#parseInt(String)
} catch (NumberFormatException nfe) {
return false;
}
int value = Integer.parseInt(s);
return value > 0 && !s.startsWith("+"); // "+1" is successfully parsed by Integer#parseInt(String)
}
}
4 changes: 2 additions & 2 deletions src/main/java/seedu/address/logic/Messages.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
*/
public class Messages {

public static final String MESSAGE_UNKNOWN_COMMAND = "Unknown command";
public static final String MESSAGE_UNKNOWN_COMMAND = "Unknown command.";
public static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format! \n%1$s";
public static final String MESSAGE_INVALID_PERSON_DISPLAYED_INDEX = "The person index provided is invalid";
public static final String MESSAGE_INVALID_PERSON_DISPLAYED_INDEX = "The person index provided is invalid.";
public static final String MESSAGE_INVALID_NOTE_DISPLAYED_INDEX = "This note index is invalid!";
public static final String MESSAGE_PERSONS_LISTED_OVERVIEW = "%1$d persons listed!";
public static final String MESSAGE_DUPLICATE_FIELDS =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class AddNoteCommand extends Command {
+ "identified by the index number used in the last person listing. "
+ "Parameters: INDEX (must be a positive integer) NOTE_CONTENT\n"
+ "Example: " + COMMAND_WORD + " 1 This is a sample note for the person.";
public static final String MESSAGE_NOTE_SUCCESS = "Added note to person.";
public static final String MESSAGE_NOTE_SUCCESS = "Added note to person: %s";
private final Index index;
private final Note note;

Expand All @@ -44,7 +44,7 @@ public CommandResult execute(Model model) throws CommandException {
}
Person p = lastShownList.get(index.getZeroBased());
p.addNote(note);
return new CommandResult(String.format(MESSAGE_NOTE_SUCCESS, Messages.format(p)));
return new CommandResult(String.format(MESSAGE_NOTE_SUCCESS, p.getName()));
}

@Override
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/seedu/address/logic/commands/EditCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import seedu.address.logic.commands.exceptions.CommandException;
import seedu.address.model.Model;
import seedu.address.model.person.Address;
import seedu.address.model.person.Avatar;
import seedu.address.model.person.Balance;
import seedu.address.model.person.Birthday;
import seedu.address.model.person.Email;
Expand Down Expand Up @@ -128,10 +129,11 @@ private static Person createEditedPerson(Person personToEdit, EditPersonDescript
Set<Tag> updatedTags = editPersonDescriptor.getTags().orElse(personToEdit.getTags());
Optional<Integer> id = personToEdit.getId();
ObservableList<Note> notes = personToEdit.getNotes();
Avatar avatar = personToEdit.getAvatar();
Balance balance = personToEdit.getBalance();

Person updatedPerson = new Person(updatedName, updatedPhone, updatedEmail, updatedAddress, updatedBirthday,
updatedLinkedin, updatedSecondaryEmail, updatedTelegram, updatedTags, id, notes, balance);
updatedLinkedin, updatedSecondaryEmail, updatedTelegram, updatedTags, id, avatar, notes, balance);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!


// Records down the alternative contact fields that are initially empty.
ArrayList<String> emptyAlternativeContactFields = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,7 @@ public AddAltCommand parse(String args) throws ParseException {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddAltCommand.MESSAGE_USAGE));
}

try {
index = ParserUtil.parseIndex(preamble);
} catch (ParseException pe) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddAltCommand.MESSAGE_USAGE));
}

index = ParserUtil.parseIndex(preamble);
argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_LINKEDIN, PREFIX_SECONDARY_EMAIL,
PREFIX_TELEGRAM, PREFIX_BIRTHDAY);
AddAltPersonDescriptor addAltPersonDescriptor = new AddAltPersonDescriptor();
Expand Down
31 changes: 19 additions & 12 deletions src/main/java/seedu/address/logic/parser/AddNoteCommandParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,33 @@
*/
public class AddNoteCommandParser implements Parser<AddNoteCommand> {

public static final String MESSAGE_EMPTY_NOTE = "NOTE_CONTENT cannot be empty!";

/**
* Parses the given {@code String} of arguments in the context of the NoteCommand
* and returns a NoteCommand object for execution.
* @throws ParseException if the user input does not conform to the expected format
*/
public AddNoteCommand parse(String args) throws ParseException {
try {
// Split based on space
String[] splitArgs = args.trim().split("\\s", 2);

if (splitArgs.length < 2) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddNoteCommand.MESSAGE_USAGE));
}
// Split based on space
String[] splitArgs = args.trim().split("\\s", 2);

Index index = ParserUtil.parseIndex(splitArgs[0]);
Note note = new Note(splitArgs[1].trim());
// Empty args
if (splitArgs[0].isEmpty()) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddNoteCommand.MESSAGE_USAGE));
}

return new AddNoteCommand(index, note);
} catch (ParseException pe) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddNoteCommand.MESSAGE_USAGE), pe);
// Only 1 arg
if (splitArgs.length == 1) {
// Assume that user follows command format so first argument by default is index.
ParserUtil.parseIndex(splitArgs[0]);
// If user keys in valid index, will then subsequently throw exception; notes cannot be whitespace/empty.
throw new ParseException(MESSAGE_EMPTY_NOTE);
}

Index index = ParserUtil.parseIndex(splitArgs[0]);
Note note = new Note(splitArgs[1]);

return new AddNoteCommand(index, note);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package seedu.address.logic.parser;

import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;

import seedu.address.commons.core.index.Index;
import seedu.address.logic.commands.DeleteCommand;
import seedu.address.logic.parser.exceptions.ParseException;
Expand All @@ -17,13 +15,8 @@ public class DeleteCommandParser implements Parser<DeleteCommand> {
* @throws ParseException if the user input does not conform the expected format
*/
public DeleteCommand parse(String args) throws ParseException {
try {
Index index = ParserUtil.parseIndex(args);
return new DeleteCommand(index);
} catch (ParseException pe) {
throw new ParseException(
String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE), pe);
}
Index index = ParserUtil.parseIndex(args);
return new DeleteCommand(index);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,7 @@ public EditCommand parse(String args) throws ParseException {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditCommand.MESSAGE_USAGE));
}

try {
index = ParserUtil.parseIndex(preamble);
} catch (ParseException pe) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditCommand.MESSAGE_USAGE));
}

index = ParserUtil.parseIndex(preamble);
argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_BIRTHDAY, PREFIX_SECONDARY_EMAIL, PREFIX_TELEGRAM, PREFIX_LINKEDIN);

Expand Down
17 changes: 13 additions & 4 deletions src/main/java/seedu/address/logic/parser/ParserUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
public class ParserUtil {

public static final String MESSAGE_INVALID_INDEX = "Index is not a non-zero unsigned integer.";
public static final String MESSAGE_NOT_A_INDEX = "Index should be an integer.";

/**
* Parses {@code oneBasedIndex} into an {@code Index} and returns it. Leading and trailing whitespaces will be
Expand All @@ -36,10 +37,15 @@ public class ParserUtil {
*/
public static Index parseIndex(String oneBasedIndex) throws ParseException {
String trimmedIndex = oneBasedIndex.trim();
if (!StringUtil.isNonZeroUnsignedInteger(trimmedIndex)) {
throw new ParseException(MESSAGE_INVALID_INDEX);
try {
boolean isValidIndex = StringUtil.isNonZeroUnsignedInteger(trimmedIndex);
if (!isValidIndex) {
throw new ParseException(MESSAGE_INVALID_INDEX);
}
return Index.fromOneBased(Integer.parseInt(trimmedIndex));
} catch (NumberFormatException e) {
throw new ParseException(MESSAGE_NOT_A_INDEX);
}
return Index.fromOneBased(Integer.parseInt(trimmedIndex));
}

/**
Expand Down Expand Up @@ -170,7 +176,10 @@ public static Set<Tag> parseTags(Collection<String> tags) throws ParseException
requireNonNull(tags);
final Set<Tag> tagSet = new HashSet<>();
for (String tagName : tags) {
tagSet.add(parseTag(tagName));
Tag tagToAdd = parseTag(tagName);
if (!tagSet.contains(tagToAdd)) {
tagSet.add(tagToAdd);
}
}
return tagSet;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,17 @@ public class RemoveNoteCommandParser implements Parser<RemoveNoteCommand> {
* @throws ParseException if the user input does not conform to the expected format
*/
public RemoveNoteCommand parse(String args) throws ParseException {
try {
// Split based on space
String[] splitArgs = args.trim().split("\\s");
// Split based on space
String[] splitArgs = args.trim().split("\\s");

if (splitArgs.length != 2) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT,
RemoveNoteCommand.MESSAGE_USAGE));
}

Index indexPerson = ParserUtil.parseIndex(splitArgs[0]);
Index indexNote = ParserUtil.parseIndex(splitArgs[1]);

return new RemoveNoteCommand(indexPerson, indexNote);
} catch (ParseException pe) {
if (splitArgs.length != 2) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT,
RemoveNoteCommand.MESSAGE_USAGE), pe);
RemoveNoteCommand.MESSAGE_USAGE));
}

Index indexPerson = ParserUtil.parseIndex(splitArgs[0]);
Index indexNote = ParserUtil.parseIndex(splitArgs[1]);

return new RemoveNoteCommand(indexPerson, indexNote);
}
}
6 changes: 3 additions & 3 deletions src/main/java/seedu/address/model/tag/Tag.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public enum EmergencyTags {
*/
public static boolean isEmergencyTag(String tagName) {
for (EmergencyTags tag : EmergencyTags.values()) {
if (tag.name().equals(tagName)) {
if (tag.name().equals(tagName.toUpperCase())) {
return true;
}
}
Expand Down Expand Up @@ -64,12 +64,12 @@ public boolean equals(Object other) {
}

Tag otherTag = (Tag) other;
return tagName.equals(otherTag.tagName);
return tagName.compareToIgnoreCase(otherTag.tagName) == 0;
}

@Override
public int hashCode() {
return tagName.hashCode();
return tagName.toUpperCase().hashCode();
}

/**
Expand Down
14 changes: 8 additions & 6 deletions src/test/java/seedu/address/commons/util/StringUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ public class StringUtilTest {
public void isNonZeroUnsignedInteger() {

// EP: empty strings
assertFalse(StringUtil.isNonZeroUnsignedInteger("")); // Boundary value
assertFalse(StringUtil.isNonZeroUnsignedInteger(" "));
assertThrows(NumberFormatException.class, () -> StringUtil.isNonZeroUnsignedInteger("")); // Boundary value
assertThrows(NumberFormatException.class, () -> StringUtil.isNonZeroUnsignedInteger(" "));

// EP: not a number
assertFalse(StringUtil.isNonZeroUnsignedInteger("a"));
assertFalse(StringUtil.isNonZeroUnsignedInteger("aaa"));
assertThrows(NumberFormatException.class, () -> StringUtil.isNonZeroUnsignedInteger("a"));
assertThrows(NumberFormatException.class, () -> StringUtil.isNonZeroUnsignedInteger("aaa"));

// EP: zero
assertFalse(StringUtil.isNonZeroUnsignedInteger("0"));
Expand All @@ -34,8 +34,10 @@ public void isNonZeroUnsignedInteger() {
assertFalse(StringUtil.isNonZeroUnsignedInteger("+1"));

// EP: numbers with white space
assertFalse(StringUtil.isNonZeroUnsignedInteger(" 10 ")); // Leading/trailing spaces
assertFalse(StringUtil.isNonZeroUnsignedInteger("1 0")); // Spaces in the middle
assertThrows(NumberFormatException.class, () ->
StringUtil.isNonZeroUnsignedInteger(" 10 ")); // Leading/trailing spaces
assertThrows(NumberFormatException.class, () ->
StringUtil.isNonZeroUnsignedInteger("1 0")); // Spaces in the middle

// EP: number larger than Integer.MAX_VALUE
assertFalse(StringUtil.isNonZeroUnsignedInteger(Long.toString(Integer.MAX_VALUE + 1)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public void execute_outOfBoundIndexInNonEmptyAddressBook_exceptionThrown() {
} catch (ParseException e) {
fail();
} catch (CommandException e) {
assertEquals(e.getMessage(), "The person index provided is invalid");
assertEquals(e.getMessage(), "The person index provided is invalid.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,18 @@ public void parse_invalidFormat_failure() {
assertParseFailure(parser, "1 b/24/07 b/25/07", Messages.MESSAGE_DUPLICATE_FIELDS + "b/");

// Invalid prefix
assertParseFailure(parser, "1 i/string b/24/07", MESSAGE_INVALID_FORMAT);
assertParseFailure(parser, "1 i/string b/24/07", ParserUtil.MESSAGE_NOT_A_INDEX);
}

@Test
public void parse_invalidPreamble_failure() {
// Heuristic: Boundary value analysis

// One value below boundary
assertParseFailure(parser, "-1 b/24/07/01" , MESSAGE_INVALID_FORMAT);
assertParseFailure(parser, "-1 b/24/07/01" , ParserUtil.MESSAGE_INVALID_INDEX);

// Value at boundary
assertParseFailure(parser, "0 b/24/07/01" , MESSAGE_INVALID_FORMAT);
assertParseFailure(parser, "0 b/24/07/01" , ParserUtil.MESSAGE_INVALID_INDEX);
}

}
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package seedu.address.logic.parser;

import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
import static seedu.address.logic.parser.CommandParserTestUtil.assertParseFailure;
import static seedu.address.logic.parser.CommandParserTestUtil.assertParseSuccess;

import org.junit.jupiter.api.Test;

import seedu.address.commons.core.index.Index;
import seedu.address.logic.Messages;
import seedu.address.logic.commands.AddNoteCommand;
import seedu.address.model.person.Note;

Expand All @@ -21,12 +21,14 @@ public void parse_allFieldsPresent_success() {

@Test
public void parse_missingDetails_failure() {
// No arguments
assertParseFailure(parser, "",
String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddNoteCommand.MESSAGE_USAGE));

// Missing note details
assertParseFailure(parser, "1",
String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, AddNoteCommand.MESSAGE_USAGE));
assertParseFailure(parser, "1", AddNoteCommandParser.MESSAGE_EMPTY_NOTE);

// Missing index
assertParseFailure(parser, "Test note",
String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, AddNoteCommand.MESSAGE_USAGE));
assertParseFailure(parser, "Test note", ParserUtil.MESSAGE_NOT_A_INDEX);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package seedu.address.logic.parser;

import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
import static seedu.address.logic.parser.CommandParserTestUtil.assertParseFailure;
import static seedu.address.logic.parser.CommandParserTestUtil.assertParseSuccess;
import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON;
Expand All @@ -27,6 +26,6 @@ public void parse_validArgs_returnsDeleteCommand() {

@Test
public void parse_invalidArgs_throwsParseException() {
assertParseFailure(parser, "a", String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE));
assertParseFailure(parser, "a", ParserUtil.MESSAGE_NOT_A_INDEX);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ public void parse_missingParts_failure() {
@Test
public void parse_invalidPreamble_failure() {
// negative index
assertParseFailure(parser, "-5" + NAME_DESC_AMY, MESSAGE_INVALID_FORMAT);
assertParseFailure(parser, "-5" + NAME_DESC_AMY, ParserUtil.MESSAGE_INVALID_INDEX);

// zero index
assertParseFailure(parser, "0" + NAME_DESC_AMY, MESSAGE_INVALID_FORMAT);
assertParseFailure(parser, "0" + NAME_DESC_AMY, ParserUtil.MESSAGE_INVALID_INDEX);

// invalid arguments being parsed as preamble
assertParseFailure(parser, "1 some random string", MESSAGE_INVALID_FORMAT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public void parse_validArgs_returnsOweCommand() {
public void parse_invalidArgs_throwsParseException() {
// Test with invalid index
assertParseFailure(parser, "a 2.50",
ParserUtil.MESSAGE_INVALID_INDEX);
ParserUtil.MESSAGE_NOT_A_INDEX);

// Test with invalid negative values
assertParseFailure(parser, "1 -2.50", Balance.MESSAGE_CONSTRAINTS);
Expand Down
Loading
Loading