From d6cd1c6d4b469e4f9d0ee8fffd2365ec431f00c6 Mon Sep 17 00:00:00 2001 From: Matthew Alexander Date: Thu, 27 Sep 2018 16:53:39 +0800 Subject: [PATCH 1/2] Delete by Name feature, solved CS2113T Bug of deleting before lisitng --- src/seedu/addressbook/commands/Command.java | 16 ++++++++++++++-- .../addressbook/commands/DeleteCommand.java | 8 ++++++-- src/seedu/addressbook/parser/Parser.java | 17 ++++++++++++++--- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/seedu/addressbook/commands/Command.java b/src/seedu/addressbook/commands/Command.java index a54cbcb5b..3cba396e6 100644 --- a/src/seedu/addressbook/commands/Command.java +++ b/src/seedu/addressbook/commands/Command.java @@ -2,9 +2,12 @@ import seedu.addressbook.common.Messages; import seedu.addressbook.data.AddressBook; -import seedu.addressbook.data.person.ReadOnlyPerson; +import seedu.addressbook.data.person.*; +import seedu.addressbook.data.tag.Tag; import java.util.List; +import java.util.NoSuchElementException; +import java.util.Set; import static seedu.addressbook.ui.Gui.DISPLAYED_INDEX_OFFSET; @@ -51,7 +54,7 @@ public CommandResult execute(){ */ public void setData(AddressBook addressBook, List relevantPersons) { this.addressBook = addressBook; - this.relevantPersons = relevantPersons; + this.relevantPersons = (relevantPersons.isEmpty()) ? addressBook.getAllPersons().immutableListView() : relevantPersons; } /** @@ -63,6 +66,15 @@ protected ReadOnlyPerson getTargetPerson() throws IndexOutOfBoundsException { return relevantPersons.get(getTargetIndex() - DISPLAYED_INDEX_OFFSET); } + protected ReadOnlyPerson getTargetPerson(String name) throws UniquePersonList.PersonNotFoundException { + for (ReadOnlyPerson person: relevantPersons) { + if (person.getName().toString().equals(name)) { + return person; + } + } + throw new UniquePersonList.PersonNotFoundException(); + } + public int getTargetIndex() { return targetIndex; } diff --git a/src/seedu/addressbook/commands/DeleteCommand.java b/src/seedu/addressbook/commands/DeleteCommand.java index 1dd78f85e..a259b7685 100644 --- a/src/seedu/addressbook/commands/DeleteCommand.java +++ b/src/seedu/addressbook/commands/DeleteCommand.java @@ -19,19 +19,23 @@ public class DeleteCommand extends Command { public static final String MESSAGE_DELETE_PERSON_SUCCESS = "Deleted Person: %1$s"; + private String nameToSearch = ""; public DeleteCommand(int targetVisibleIndex) { super(targetVisibleIndex); } + public DeleteCommand(String name) { + this.nameToSearch = name; + } + @Override public CommandResult execute() { try { - final ReadOnlyPerson target = getTargetPerson(); + final ReadOnlyPerson target = (nameToSearch.isEmpty()) ? getTargetPerson() : getTargetPerson(nameToSearch); addressBook.removePerson(target); return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, target)); - } catch (IndexOutOfBoundsException ie) { return new CommandResult(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); } catch (PersonNotFoundException pnfe) { diff --git a/src/seedu/addressbook/parser/Parser.java b/src/seedu/addressbook/parser/Parser.java index 58f4f7e6c..178aeb3e1 100644 --- a/src/seedu/addressbook/parser/Parser.java +++ b/src/seedu/addressbook/parser/Parser.java @@ -26,6 +26,7 @@ public class Parser { + " (?p?)a/(?
[^/]+)" + "(?(?: t/[^/]+)*)"); // variable number of tags + public static final Pattern PERSON_NAME_FORMAT = Pattern.compile("(?[^/]+)"); /** * Signals that the user input could not be parsed. @@ -149,9 +150,12 @@ private static Set getTagsFromArgs(String tagArguments) throws IllegalVa */ private Command prepareDelete(String args) { try { - final int targetIndex = parseArgsAsDisplayedIndex(args); - return new DeleteCommand(targetIndex); - } catch (ParseException | NumberFormatException e) { +// final int targetIndex = parseArgsAsDisplayedIndex(args); +// return new DeleteCommand(targetIndex); + final String name = parseArgsAsName(args); + return new DeleteCommand(name); + //| NumberFormatException e + } catch (ParseException e) { return new IncorrectCommand(String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE)); } } @@ -206,6 +210,13 @@ private int parseArgsAsDisplayedIndex(String args) throws ParseException, Number return Integer.parseInt(matcher.group("targetIndex")); } + private String parseArgsAsName(String args) throws ParseException { + final Matcher matcher = PERSON_NAME_FORMAT.matcher(args.trim()); + if (!matcher.matches()) { + throw new ParseException("Could not find name to parse"); + } + return matcher.group(0); + } /** * Parses arguments in the context of the find person command. From ae276afc4b0dc8f1a2328a91ad1069506a0023d5 Mon Sep 17 00:00:00 2001 From: Matthew Alexander Date: Thu, 27 Sep 2018 17:31:17 +0800 Subject: [PATCH 2/2] Delete can be by name and index, removed test cases that prevented this --- src/seedu/addressbook/common/Utils.java | 23 +++++++++++++++++++ src/seedu/addressbook/parser/Parser.java | 9 +++++--- .../seedu/addressbook/logic/LogicTest.java | 12 +++++----- .../seedu/addressbook/parser/ParserTest.java | 12 +++++----- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/seedu/addressbook/common/Utils.java b/src/seedu/addressbook/common/Utils.java index 245f347c4..a2ce62fa4 100644 --- a/src/seedu/addressbook/common/Utils.java +++ b/src/seedu/addressbook/common/Utils.java @@ -34,4 +34,27 @@ public static boolean elementsAreUnique(Collection items) { } return true; } + + public static boolean isStringInteger(String str) { + if (str == null || str.isEmpty()) { + return false; + } + + int i = 0; + if (str.charAt(0) == '-') { + if (str.length() == 1) { + return false; + } + i = 1; + } + + while (i < str.length()) { + char c = str.charAt(i); + if (c < '0' || c > '9') { + return false; + } + i++; + } + return true; + } } diff --git a/src/seedu/addressbook/parser/Parser.java b/src/seedu/addressbook/parser/Parser.java index 825160300..f907885ba 100644 --- a/src/seedu/addressbook/parser/Parser.java +++ b/src/seedu/addressbook/parser/Parser.java @@ -1,6 +1,7 @@ package seedu.addressbook.parser; import seedu.addressbook.commands.*; +import seedu.addressbook.common.Utils; import seedu.addressbook.data.exception.IllegalValueException; import java.util.*; @@ -156,11 +157,13 @@ private static Set getTagsFromArgs(String tagArguments) throws IllegalVa */ private Command prepareDelete(String args) { try { -// final int targetIndex = parseArgsAsDisplayedIndex(args); -// return new DeleteCommand(targetIndex); final String name = parseArgsAsName(args); + if (Utils.isStringInteger(name)) { + final int targetIndex = parseArgsAsDisplayedIndex(args); + return new DeleteCommand(targetIndex); + } + return new DeleteCommand(name); - //| NumberFormatException e } catch (ParseException e) { return new IncorrectCommand(String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE)); } diff --git a/test/java/seedu/addressbook/logic/LogicTest.java b/test/java/seedu/addressbook/logic/LogicTest.java index 396fb4bc9..7636d53a8 100644 --- a/test/java/seedu/addressbook/logic/LogicTest.java +++ b/test/java/seedu/addressbook/logic/LogicTest.java @@ -331,12 +331,12 @@ public void execute_tryToViewAllPersonMissingInAddressBook_errorMessage() throws lastShownList); } - @Test - public void execute_delete_invalidArgsFormat() throws Exception { - String expectedMessage = String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE); - assertCommandBehavior("delete ", expectedMessage); - assertCommandBehavior("delete arg not number", expectedMessage); - } +// @Test +// public void execute_delete_invalidArgsFormat() throws Exception { +// String expectedMessage = String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE); +// assertCommandBehavior("delete ", expectedMessage); +// assertCommandBehavior("delete arg not number", expectedMessage); +// } @Test public void execute_delete_invalidIndex() throws Exception { diff --git a/test/java/seedu/addressbook/parser/ParserTest.java b/test/java/seedu/addressbook/parser/ParserTest.java index 5b5f5b013..5d8ff875e 100644 --- a/test/java/seedu/addressbook/parser/ParserTest.java +++ b/test/java/seedu/addressbook/parser/ParserTest.java @@ -75,12 +75,12 @@ public void deleteCommand_noArgs() { parseAndAssertIncorrectWithMessage(resultMessage, inputs); } - @Test - public void deleteCommand_argsIsNotSingleNumber() { - final String[] inputs = { "delete notAnumber ", "delete 8*wh12", "delete 1 2 3 4 5" }; - final String resultMessage = String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE); - parseAndAssertIncorrectWithMessage(resultMessage, inputs); - } +// @Test +// public void deleteCommand_argsIsNotSingleNumber() { +// final String[] inputs = { "delete notAnumber ", "delete 8*wh12", "delete 1 2 3 4 5" }; +// final String resultMessage = String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE); +// parseAndAssertIncorrectWithMessage(resultMessage, inputs); +// } @Test public void deleteCommand_numericArg_indexParsedCorrectly() {