diff --git a/src/main/java/tuteez/logic/Messages.java b/src/main/java/tuteez/logic/Messages.java index 2dd38349586..7d11282f3ee 100644 --- a/src/main/java/tuteez/logic/Messages.java +++ b/src/main/java/tuteez/logic/Messages.java @@ -15,6 +15,8 @@ public class Messages { 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_NAME = + "No student named %1$s was found, please try again!"; public static final String MESSAGE_PERSONS_LISTED_OVERVIEW = "%1$d persons listed!"; public static final String MESSAGE_DUPLICATE_FIELDS = "Multiple values specified for the following single-valued field(s): "; diff --git a/src/main/java/tuteez/logic/commands/DeleteCommand.java b/src/main/java/tuteez/logic/commands/DeleteCommand.java index e1143178556..02103dc7cfc 100644 --- a/src/main/java/tuteez/logic/commands/DeleteCommand.java +++ b/src/main/java/tuteez/logic/commands/DeleteCommand.java @@ -9,6 +9,7 @@ import tuteez.logic.Messages; import tuteez.logic.commands.exceptions.CommandException; import tuteez.model.Model; +import tuteez.model.person.Name; import tuteez.model.person.Person; /** @@ -19,32 +20,63 @@ public class DeleteCommand extends Command { public static final String COMMAND_WORD = "delete"; public static final String MESSAGE_USAGE = COMMAND_WORD - + ": Deletes the person identified by the index number used in the displayed person list.\n" - + "Parameters: INDEX (must be a positive integer)\n" - + "Example: " + COMMAND_WORD + " 1"; + + ": Deletes the person identified by the index number used in the displayed person list or by name.\n" + + "Parameters: INDEX (must be a positive integer) or NAME (must be a valid name in the addressbook)\n" + + "Example: " + COMMAND_WORD + " 1" + " or " + COMMAND_WORD + " John Doe"; public static final String MESSAGE_DELETE_PERSON_SUCCESS = "Deleted Person: %1$s"; private final Index targetIndex; + private final Name targetName; + /** + * Creates a DeleteCommand to delete the person at the specified {@code targetIndex} of the displayed list. + * @param targetIndex the index of the person in the displayed list to delete + */ public DeleteCommand(Index targetIndex) { this.targetIndex = targetIndex; + this.targetName = null; + } + + /** + * Creates a DeleteCommand to delete the person with the specified {@code targetName}. + * @param targetName the name of the person to delete + */ + public DeleteCommand(Name targetName) { + this.targetIndex = null; + this.targetName = targetName; } @Override public CommandResult execute(Model model) throws CommandException { requireNonNull(model); - List lastShownList = model.getFilteredPersonList(); - - if (targetIndex.getZeroBased() >= lastShownList.size()) { - throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); + Person personToDelete; + if (targetIndex != null) { + personToDelete = getPersonToDeleteByIndex(model, targetIndex); + } else { + personToDelete = getPersonToDeleteByName(model, targetName); } - Person personToDelete = lastShownList.get(targetIndex.getZeroBased()); model.deletePerson(personToDelete); return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, Messages.format(personToDelete))); } + private Person getPersonToDeleteByIndex(Model model, Index index) throws CommandException { + List lastShownList = model.getFilteredPersonList(); + if (index.getZeroBased() >= lastShownList.size()) { + throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); + } + return lastShownList.get(index.getZeroBased()); + } + + private Person getPersonToDeleteByName(Model model, Name name) throws CommandException { + Person personToDelete = model.findPersonByName(name); + if (personToDelete == null) { + throw new CommandException(String.format(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_NAME, name)); + } + return personToDelete; + } + @Override public boolean equals(Object other) { if (other == this) { @@ -57,13 +89,16 @@ public boolean equals(Object other) { } DeleteCommand otherDeleteCommand = (DeleteCommand) other; - return targetIndex.equals(otherDeleteCommand.targetIndex); + boolean isIndexValid = targetIndex != null && targetIndex.equals(otherDeleteCommand.targetIndex); + boolean isNameValid = targetName != null && targetName.equals(otherDeleteCommand.targetName); + return isIndexValid || isNameValid; } @Override public String toString() { return new ToStringBuilder(this) .add("targetIndex", targetIndex) + .add("targetName", targetName) .toString(); } } diff --git a/src/main/java/tuteez/logic/parser/CliSyntax.java b/src/main/java/tuteez/logic/parser/CliSyntax.java index fa7e787a2f6..86e3a96dbce 100644 --- a/src/main/java/tuteez/logic/parser/CliSyntax.java +++ b/src/main/java/tuteez/logic/parser/CliSyntax.java @@ -11,5 +11,4 @@ public class CliSyntax { public static final Prefix PREFIX_EMAIL = new Prefix("e/"); public static final Prefix PREFIX_ADDRESS = new Prefix("a/"); public static final Prefix PREFIX_TAG = new Prefix("t/"); - } diff --git a/src/main/java/tuteez/logic/parser/DeleteCommandParser.java b/src/main/java/tuteez/logic/parser/DeleteCommandParser.java index 0d0109a168e..d7d1db5bf54 100644 --- a/src/main/java/tuteez/logic/parser/DeleteCommandParser.java +++ b/src/main/java/tuteez/logic/parser/DeleteCommandParser.java @@ -5,6 +5,7 @@ import tuteez.commons.core.index.Index; import tuteez.logic.commands.DeleteCommand; import tuteez.logic.parser.exceptions.ParseException; +import tuteez.model.person.Name; /** * Parses input arguments and creates a new DeleteCommand object @@ -18,12 +19,16 @@ public class DeleteCommandParser implements Parser { */ public DeleteCommand parse(String args) throws ParseException { try { - Index index = ParserUtil.parseIndex(args); - return new DeleteCommand(index); + if (args.trim().matches("\\d+")) { + Index index = ParserUtil.parseIndex(args); + return new DeleteCommand(index); + } else { + Name name = ParserUtil.parseName(args); + return new DeleteCommand(name); + } } catch (ParseException pe) { throw new ParseException( String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE), pe); } } - } diff --git a/src/main/java/tuteez/model/AddressBook.java b/src/main/java/tuteez/model/AddressBook.java index 0cfc067184e..06824dab6d9 100644 --- a/src/main/java/tuteez/model/AddressBook.java +++ b/src/main/java/tuteez/model/AddressBook.java @@ -6,6 +6,7 @@ import javafx.collections.ObservableList; import tuteez.commons.util.ToStringBuilder; +import tuteez.model.person.Name; import tuteez.model.person.Person; import tuteez.model.person.UniquePersonList; @@ -94,6 +95,18 @@ public void removePerson(Person key) { persons.remove(key); } + /** + * Finds and returns the {@code person} from this {@code AddressBook} with the specified {@code Name}. + * {@code Name} must exist in the address book. + */ + public Person findPersonByName(Name targetName) { + requireNonNull(targetName); + return getPersonList().stream() + .filter(person -> person.getName().fullName.equalsIgnoreCase(targetName.fullName)) + .findFirst() + .orElse(null); + } + //// util methods @Override diff --git a/src/main/java/tuteez/model/Model.java b/src/main/java/tuteez/model/Model.java index a073a48c156..df4b6dcb71a 100644 --- a/src/main/java/tuteez/model/Model.java +++ b/src/main/java/tuteez/model/Model.java @@ -5,6 +5,7 @@ import javafx.collections.ObservableList; import tuteez.commons.core.GuiSettings; +import tuteez.model.person.Name; import tuteez.model.person.Person; /** @@ -84,4 +85,9 @@ public interface Model { * @throws NullPointerException if {@code predicate} is null. */ void updateFilteredPersonList(Predicate predicate); + + /** + * Returns the {@code person} in the address book with the given name. + */ + Person findPersonByName(Name name); } diff --git a/src/main/java/tuteez/model/ModelManager.java b/src/main/java/tuteez/model/ModelManager.java index 0e4f6461981..35f50a64083 100644 --- a/src/main/java/tuteez/model/ModelManager.java +++ b/src/main/java/tuteez/model/ModelManager.java @@ -11,6 +11,7 @@ import javafx.collections.transformation.FilteredList; import tuteez.commons.core.GuiSettings; import tuteez.commons.core.LogsCenter; +import tuteez.model.person.Name; import tuteez.model.person.Person; /** @@ -111,6 +112,12 @@ public void setPerson(Person target, Person editedPerson) { addressBook.setPerson(target, editedPerson); } + @Override + public Person findPersonByName(Name targetName) { + requireNonNull(targetName); + return addressBook.findPersonByName(targetName); + } + //=========== Filtered Person List Accessors ============================================================= /** diff --git a/src/test/java/tuteez/logic/commands/AddCommandTest.java b/src/test/java/tuteez/logic/commands/AddCommandTest.java index bdd9800f2b3..c2fc556c3a6 100644 --- a/src/test/java/tuteez/logic/commands/AddCommandTest.java +++ b/src/test/java/tuteez/logic/commands/AddCommandTest.java @@ -22,6 +22,7 @@ import tuteez.model.Model; import tuteez.model.ReadOnlyAddressBook; import tuteez.model.ReadOnlyUserPrefs; +import tuteez.model.person.Name; import tuteez.model.person.Person; import tuteez.testutil.PersonBuilder; @@ -157,6 +158,11 @@ public ObservableList getFilteredPersonList() { public void updateFilteredPersonList(Predicate predicate) { throw new AssertionError("This method should not be called."); } + + @Override + public Person findPersonByName(Name name) { + throw new AssertionError("This method should not be called."); + } } /** diff --git a/src/test/java/tuteez/logic/commands/DeleteCommandTest.java b/src/test/java/tuteez/logic/commands/DeleteCommandTest.java index eed65be9f4c..711aa28a763 100644 --- a/src/test/java/tuteez/logic/commands/DeleteCommandTest.java +++ b/src/test/java/tuteez/logic/commands/DeleteCommandTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static tuteez.logic.commands.CommandTestUtil.VALID_NAME_AMY; import static tuteez.logic.commands.CommandTestUtil.assertCommandFailure; import static tuteez.logic.commands.CommandTestUtil.assertCommandSuccess; import static tuteez.logic.commands.CommandTestUtil.showPersonAtIndex; @@ -17,7 +18,9 @@ import tuteez.model.Model; import tuteez.model.ModelManager; import tuteez.model.UserPrefs; +import tuteez.model.person.Name; import tuteez.model.person.Person; +import tuteez.testutil.PersonBuilder; /** * Contains integration tests (interaction with the Model) and unit tests for @@ -79,6 +82,31 @@ public void execute_invalidIndexFilteredList_throwsCommandException() { assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); } + @Test + public void execute_validName_success() { + Person personAdded = new PersonBuilder().withName(VALID_NAME_AMY).build(); + model.addPerson(personAdded); + Name targetName = personAdded.getName(); + Person personToDelete = model.findPersonByName(targetName); + DeleteCommand deleteCommand = new DeleteCommand(targetName); + String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_PERSON_SUCCESS, + Messages.format(personToDelete)); + Model expectedModel = new ModelManager(model.getAddressBook(), new UserPrefs()); + expectedModel.deletePerson(personToDelete); + assertCommandSuccess(deleteCommand, model, expectedMessage, expectedModel); + } + + @Test + public void execute_invalidName_throwsCommandException() { + Person personAdded = new PersonBuilder().withName(VALID_NAME_AMY).build(); + model.addPerson(personAdded); + Name invalidName = new Name("Amyy Beee"); + DeleteCommand deleteCommand = new DeleteCommand(invalidName); + String expectedMessage = String.format(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_NAME, + invalidName); + assertCommandFailure(deleteCommand, model, expectedMessage); + } + @Test public void equals() { DeleteCommand deleteFirstCommand = new DeleteCommand(INDEX_FIRST_PERSON); @@ -104,9 +132,16 @@ public void equals() { @Test public void toStringMethod() { Index targetIndex = Index.fromOneBased(1); - DeleteCommand deleteCommand = new DeleteCommand(targetIndex); - String expected = DeleteCommand.class.getCanonicalName() + "{targetIndex=" + targetIndex + "}"; - assertEquals(expected, deleteCommand.toString()); + DeleteCommand deleteCommandWithIndex = new DeleteCommand(targetIndex); + String expectedWithIndex = DeleteCommand.class.getCanonicalName() + + "{targetIndex=" + targetIndex + ", targetName=null}"; + assertEquals(expectedWithIndex, deleteCommandWithIndex.toString()); + + Name targetName = new Name("Mary Jane"); + DeleteCommand deleteCommandWithName = new DeleteCommand(targetName); + String expectedWithName = DeleteCommand.class.getCanonicalName() + + "{targetIndex=null, targetName=" + targetName + "}"; + assertEquals(expectedWithName, deleteCommandWithName.toString()); } /** diff --git a/src/test/java/tuteez/logic/parser/DeleteCommandParserTest.java b/src/test/java/tuteez/logic/parser/DeleteCommandParserTest.java index 43f74102dac..638ddc0fa53 100644 --- a/src/test/java/tuteez/logic/parser/DeleteCommandParserTest.java +++ b/src/test/java/tuteez/logic/parser/DeleteCommandParserTest.java @@ -8,6 +8,7 @@ import org.junit.jupiter.api.Test; import tuteez.logic.commands.DeleteCommand; +import tuteez.model.person.Name; /** * As we are only doing white-box testing, our test cases do not cover path variations @@ -21,12 +22,17 @@ public class DeleteCommandParserTest { private DeleteCommandParser parser = new DeleteCommandParser(); @Test - public void parse_validArgs_returnsDeleteCommand() { + public void parse_validArgsWithIndex_returnsDeleteCommand() { assertParseSuccess(parser, "1", new DeleteCommand(INDEX_FIRST_PERSON)); } + @Test + public void parse_validArgsWithName_returnsDeleteCommand() { + assertParseSuccess(parser, "Alice", new DeleteCommand(new Name("Alice"))); + } + @Test public void parse_invalidArgs_throwsParseException() { - assertParseFailure(parser, "a", String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE)); + assertParseFailure(parser, "-1", String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE)); } } diff --git a/src/test/java/tuteez/model/ModelManagerTest.java b/src/test/java/tuteez/model/ModelManagerTest.java index 69c83ae037a..d2f815c44b7 100644 --- a/src/test/java/tuteez/model/ModelManagerTest.java +++ b/src/test/java/tuteez/model/ModelManagerTest.java @@ -88,6 +88,12 @@ public void hasPerson_personInAddressBook_returnsTrue() { assertTrue(modelManager.hasPerson(ALICE)); } + @Test + public void findPersonByName_nameNotInAddressBook_returnsNull() { + modelManager.addPerson(ALICE); + assertEquals(null, modelManager.findPersonByName(BENSON.getName())); + } + @Test public void getFilteredPersonList_modifyList_throwsUnsupportedOperationException() { assertThrows(UnsupportedOperationException.class, () -> modelManager.getFilteredPersonList().remove(0));