Merge pull request nus-cs2103-AY2425S1#57 from kaikquah/branch-Update…

…-DeleteCommand

Update `DeleteCommand` to accept the `Name` of a `Person`

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): ";

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<Person> 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<Person> 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();
     }
 }

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/");
-
 }

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<DeleteCommand> {
      */
     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);
         }
     }
-
 }

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

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<Person> predicate);
+
+    /**
+     * Returns the {@code person} in the address book with the given name.
+     */
+    Person findPersonByName(Name name);
 }

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 =============================================================
 
     /**

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<Person> getFilteredPersonList() {
         public void updateFilteredPersonList(Predicate<Person> 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.");
+        }
     }
 
     /**

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());
     }
 
     /**

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));
     }
 }

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));