Merge pull request AY2425S1-CS2103T-F15-2#47 from jessica2828/delete-…

…command

Update Delete Command

src/main/java/seedu/address/logic/commands/AddCommand.java

@@ -32,13 +32,13 @@ public class AddCommand extends Command {
             + PREFIX_COURSE + "COURSE "
             + PREFIX_TAG + "TAG\n"
             + "Example: " + COMMAND_WORD + " "
-            + PREFIX_STUDENTID + "12345678"
+            + PREFIX_STUDENTID + "12345678 "
             + PREFIX_NAME + "John Doe "
             + PREFIX_PHONE + "98765432 "
             + PREFIX_EMAIL + "[email protected] "
             + PREFIX_ADDRESS + "311, Clementi Ave 2, #02-25 "
-            + PREFIX_COURSE + "Computer Science"
-            + PREFIX_TAG + "Student";
+            + PREFIX_COURSE + "Computer Science "
+            + PREFIX_TAG + "Student ";
 
 
     public static final String MESSAGE_SUCCESS = "New person added: %1$s";

src/main/java/seedu/address/logic/commands/DeleteCommand.java

@@ -1,15 +1,16 @@
 package seedu.address.logic.commands;
 
 import static java.util.Objects.requireNonNull;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_STUDENTID;
 
 import java.util.List;
 
-import seedu.address.commons.core.index.Index;
 import seedu.address.commons.util.ToStringBuilder;
 import seedu.address.logic.Messages;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
 import seedu.address.model.person.Person;
+import seedu.address.model.person.StudentId;
 
 /**
  * Deletes a person identified using it's displayed index from the address book.
@@ -19,30 +20,53 @@ 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";
-
-    public static final String MESSAGE_DELETE_PERSON_SUCCESS = "Deleted Person: %1$s";
-
-    private final Index targetIndex;
-
-    public DeleteCommand(Index targetIndex) {
-        this.targetIndex = targetIndex;
+            + ": Deletes the student identified by the Student ID used in the displayed person list.\n"
+            + "Parameters: "
+            + PREFIX_STUDENTID + "ID\n"
+            + "Example: " + COMMAND_WORD + " "
+            + PREFIX_STUDENTID + "12345678";
+
+    public static final String MESSAGE_DELETE_PERSON_SUCCESS = "Deleted Student: %1$s";
+    public static final String MESSAGE_PERSON_NOT_FOUND = "No student is found with Student ID: %1$s";
+    private final StudentId studentId;
+
+    /**
+     * Creates a DeleteCommand to delete the person identified by the specified {@code StudentId}.
+     *
+     * @param studentId The student ID of the person to be deleted.
+     * @throws NullPointerException if the {@code studentId} is null.
+     */
+    public DeleteCommand(StudentId studentId) {
+        requireNonNull(studentId);
+        this.studentId = studentId;
     }
 
+    /**
+     * Executes the delete command and removes a person identified by the given studentID.
+     *
+     * @param model the model that contains the data of persons
+     * @return a CommandResult that shows the outcome of the command
+     * @throws CommandException if the studentID is invalid or not found
+     */
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
         List<Person> lastShownList = model.getFilteredPersonList();
+        Person toDelete = null;
+
+        for (Person person : lastShownList) {
+            if (person.getStudentId().equals(studentId)) {
+                toDelete = person;
+                break;
+            }
+        }
 
-        if (targetIndex.getZeroBased() >= lastShownList.size()) {
-            throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
+        if (toDelete == null) {
+            throw new CommandException(String.format(MESSAGE_PERSON_NOT_FOUND, studentId));
         }
 
-        Person personToDelete = lastShownList.get(targetIndex.getZeroBased());
-        model.deletePerson(personToDelete);
-        return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, Messages.format(personToDelete)));
+        model.deletePerson(toDelete);
+        return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, Messages.format(toDelete)));
     }
 
     @Override
@@ -57,13 +81,13 @@ public boolean equals(Object other) {
         }
 
         DeleteCommand otherDeleteCommand = (DeleteCommand) other;
-        return targetIndex.equals(otherDeleteCommand.targetIndex);
+        return studentId.equals(otherDeleteCommand.studentId);
     }
 
     @Override
     public String toString() {
         return new ToStringBuilder(this)
-                .add("targetIndex", targetIndex)
+                .add("targetStudentId", studentId)
                 .toString();
     }
 }

src/main/java/seedu/address/logic/parser/DeleteCommandParser.java

@@ -2,9 +2,9 @@
 
 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;
+import seedu.address.model.person.StudentId;
 
 /**
  * Parses input arguments and creates a new DeleteCommand object
@@ -17,13 +17,20 @@ 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 {
+        String trimmedArg = args.trim();
         try {
-            Index index = ParserUtil.parseIndex(args);
-            return new DeleteCommand(index);
-        } catch (ParseException pe) {
+            if (!trimmedArg.startsWith("id/")) {
+                throw new ParseException(
+                        String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE));
+            }
+
+            String studentIdString = trimmedArg.substring(3).trim();
+            StudentId studentId = ParserUtil.parseStudentId(studentIdString);
+            return new DeleteCommand(studentId);
+
+        } catch (IllegalArgumentException e) {
             throw new ParseException(
-                    String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE), pe);
+                    String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE), e);
         }
     }
-
 }

src/test/java/seedu/address/logic/LogicManagerTest.java

@@ -1,7 +1,6 @@
 package seedu.address.logic;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static seedu.address.logic.Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX;
 import static seedu.address.logic.Messages.MESSAGE_UNKNOWN_COMMAND;
 import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_AMY;
 import static seedu.address.logic.commands.CommandTestUtil.COURSE_DESC_AMY;
@@ -23,6 +22,7 @@
 
 import seedu.address.logic.commands.AddCommand;
 import seedu.address.logic.commands.CommandResult;
+import seedu.address.logic.commands.DeleteCommand;
 import seedu.address.logic.commands.ListCommand;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.logic.parser.exceptions.ParseException;
@@ -63,8 +63,8 @@ public void execute_invalidCommandFormat_throwsParseException() {
 
     @Test
     public void execute_commandExecutionError_throwsCommandException() {
-        String deleteCommand = "delete 9";
-        assertCommandException(deleteCommand, MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
+        String deleteCommand = "delete id/00000000";
+        assertCommandException(deleteCommand, String.format(DeleteCommand.MESSAGE_PERSON_NOT_FOUND, "00000000"));
     }
 
     @Test

src/test/java/seedu/address/logic/commands/DeleteCommandTest.java

@@ -12,12 +12,13 @@
 
 import org.junit.jupiter.api.Test;
 
-import seedu.address.commons.core.index.Index;
+//import seedu.address.commons.core.index.Index;
 import seedu.address.logic.Messages;
 import seedu.address.model.Model;
 import seedu.address.model.ModelManager;
 import seedu.address.model.UserPrefs;
 import seedu.address.model.person.Person;
+import seedu.address.model.person.StudentId;
 
 /**
  * Contains integration tests (interaction with the Model) and unit tests for
@@ -28,9 +29,10 @@ public class DeleteCommandTest {
     private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs());
 
     @Test
-    public void execute_validIndexUnfilteredList_success() {
+    public void execute_validStudentIdUnfilteredList_success() {
         Person personToDelete = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased());
-        DeleteCommand deleteCommand = new DeleteCommand(INDEX_FIRST_PERSON);
+        StudentId studentIdToDelete = personToDelete.getStudentId();
+        DeleteCommand deleteCommand = new DeleteCommand(studentIdToDelete);
 
         String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_PERSON_SUCCESS,
                 Messages.format(personToDelete));
@@ -42,19 +44,20 @@ public void execute_validIndexUnfilteredList_success() {
     }
 
     @Test
-    public void execute_invalidIndexUnfilteredList_throwsCommandException() {
-        Index outOfBoundIndex = Index.fromOneBased(model.getFilteredPersonList().size() + 1);
-        DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex);
-
-        assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
+    public void execute_invalidStudentIdUnfilteredList_throwsCommandException() {
+        StudentId invalidStudentId = new StudentId("12345679");
+        DeleteCommand deleteCommand = new DeleteCommand(invalidStudentId);
+        assertCommandFailure(
+                deleteCommand, model, String.format(DeleteCommand.MESSAGE_PERSON_NOT_FOUND, invalidStudentId));
     }
 
     @Test
-    public void execute_validIndexFilteredList_success() {
+    public void execute_validStudentIdFilteredList_success() {
         showPersonAtIndex(model, INDEX_FIRST_PERSON);
 
         Person personToDelete = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased());
-        DeleteCommand deleteCommand = new DeleteCommand(INDEX_FIRST_PERSON);
+        StudentId studentIdToDelete = personToDelete.getStudentId();
+        DeleteCommand deleteCommand = new DeleteCommand(studentIdToDelete);
 
         String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_PERSON_SUCCESS,
                 Messages.format(personToDelete));
@@ -67,28 +70,21 @@ public void execute_validIndexFilteredList_success() {
     }
 
     @Test
-    public void execute_invalidIndexFilteredList_throwsCommandException() {
-        showPersonAtIndex(model, INDEX_FIRST_PERSON);
-
-        Index outOfBoundIndex = INDEX_SECOND_PERSON;
-        // ensures that outOfBoundIndex is still in bounds of address book list
-        assertTrue(outOfBoundIndex.getZeroBased() < model.getAddressBook().getPersonList().size());
+    public void equals() {
+        Person firstPerson = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased());
+        Person secondPerson = model.getFilteredPersonList().get(INDEX_SECOND_PERSON.getZeroBased());
 
-        DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex);
+        StudentId firstStudentId = firstPerson.getStudentId();
+        StudentId secondStudentId = secondPerson.getStudentId();
 
-        assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
-    }
-
-    @Test
-    public void equals() {
-        DeleteCommand deleteFirstCommand = new DeleteCommand(INDEX_FIRST_PERSON);
-        DeleteCommand deleteSecondCommand = new DeleteCommand(INDEX_SECOND_PERSON);
+        DeleteCommand deleteFirstCommand = new DeleteCommand(firstStudentId);
+        DeleteCommand deleteSecondCommand = new DeleteCommand(secondStudentId);
 
         // same object -> returns true
         assertTrue(deleteFirstCommand.equals(deleteFirstCommand));
 
         // same values -> returns true
-        DeleteCommand deleteFirstCommandCopy = new DeleteCommand(INDEX_FIRST_PERSON);
+        DeleteCommand deleteFirstCommandCopy = new DeleteCommand(firstStudentId);
         assertTrue(deleteFirstCommand.equals(deleteFirstCommandCopy));
 
         // different types -> returns false
@@ -103,9 +99,11 @@ public void equals() {
 
     @Test
     public void toStringMethod() {
-        Index targetIndex = Index.fromOneBased(1);
-        DeleteCommand deleteCommand = new DeleteCommand(targetIndex);
-        String expected = DeleteCommand.class.getCanonicalName() + "{targetIndex=" + targetIndex + "}";
+        Person firstPerson = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased());
+        StudentId targetStudentId = firstPerson.getStudentId();
+
+        DeleteCommand deleteCommand = new DeleteCommand(targetStudentId);
+        String expected = DeleteCommand.class.getCanonicalName() + "{targetStudentId=" + targetStudentId + "}";
         assertEquals(expected, deleteCommand.toString());
     }
 

src/test/java/seedu/address/logic/parser/AddressBookParserTest.java

@@ -4,6 +4,7 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
 import static seedu.address.logic.Messages.MESSAGE_UNKNOWN_COMMAND;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_STUDENTID;
 import static seedu.address.testutil.Assert.assertThrows;
 import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON;
 
@@ -25,6 +26,7 @@
 import seedu.address.logic.parser.exceptions.ParseException;
 import seedu.address.model.person.NameContainsKeywordsPredicate;
 import seedu.address.model.person.Person;
+import seedu.address.model.person.StudentId;
 import seedu.address.testutil.EditPersonDescriptorBuilder;
 import seedu.address.testutil.PersonBuilder;
 import seedu.address.testutil.PersonUtil;
@@ -48,9 +50,10 @@ public void parseCommand_clear() throws Exception {
 
     @Test
     public void parseCommand_delete() throws Exception {
+        StudentId validStudentId = new StudentId("12345678");
         DeleteCommand command = (DeleteCommand) parser.parseCommand(
-                DeleteCommand.COMMAND_WORD + " " + INDEX_FIRST_PERSON.getOneBased());
-        assertEquals(new DeleteCommand(INDEX_FIRST_PERSON), command);
+                DeleteCommand.COMMAND_WORD + " " + PREFIX_STUDENTID + validStudentId);
+        assertEquals(new DeleteCommand(validStudentId), command);
     }
 
     @Test

src/test/java/seedu/address/logic/parser/DeleteCommandParserTest.java

@@ -3,11 +3,11 @@
 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;
 
 import org.junit.jupiter.api.Test;
 
 import seedu.address.logic.commands.DeleteCommand;
+import seedu.address.model.person.StudentId;
 
 /**
  * As we are only doing white-box testing, our test cases do not cover path variations
@@ -22,7 +22,9 @@ public class DeleteCommandParserTest {
 
     @Test
     public void parse_validArgs_returnsDeleteCommand() {
-        assertParseSuccess(parser, "1", new DeleteCommand(INDEX_FIRST_PERSON));
+        StudentId validStudentId = new StudentId("12345678");
+        String input = "id/" + validStudentId;
+        assertParseSuccess(parser, input, new DeleteCommand(validStudentId));
     }
 
     @Test