Skip to content

Commit e622ef3

Browse files
jerrylchongjerrylchong
authored
Merge pull request nus-cs2103-AY2021S1#80 from royleochan/branch-Delete-Contact
Make contact names unique and change delete to delete by name instead of index
2 parents 0fa35de + 5dd07d8 commit e622ef3

14 files changed

Lines changed: 95 additions & 43 deletions

File tree

‎src/main/java/seedu/address/commons/core/Messages.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ public class Messages {
77

88
public static final String MESSAGE_UNKNOWN_COMMAND = "Unknown command";
99
public static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format! \n%1$s";
10+
public static final String MESSAGE_INVALID_PERSON_DISPLAYED = "The person provided is not in your contacts";
1011
public static final String MESSAGE_INVALID_PERSON_DISPLAYED_INDEX = "The person index provided is invalid";
1112
public static final String MESSAGE_PERSONS_LISTED_OVERVIEW = "%1$d persons listed!";
1213

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

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,43 +3,46 @@
33
import static java.util.Objects.requireNonNull;
44

55
import java.util.List;
6+
import java.util.stream.Collectors;
67

78
import seedu.address.commons.core.Messages;
8-
import seedu.address.commons.core.index.Index;
99
import seedu.address.logic.commands.exceptions.CommandException;
1010
import seedu.address.model.Model;
11+
import seedu.address.model.person.Name;
1112
import seedu.address.model.person.Person;
1213

1314
/**
1415
* Deletes a person identified using it's displayed index from the address book.
1516
*/
1617
public class DeleteCommand extends Command {
1718

18-
public static final String COMMAND_WORD = "delete";
19+
public static final String COMMAND_WORD = "contact delete";
1920

2021
public static final String MESSAGE_USAGE = COMMAND_WORD
21-
+ ": Deletes the person identified by the index number used in the displayed person list.\n"
22-
+ "Parameters: INDEX (must be a positive integer)\n"
23-
+ "Example: " + COMMAND_WORD + " 1";
22+
+ ": Deletes the person identified by the name used in the displayed person list.\n"
23+
+ "Parameters: NAME (must be a valid name)\n"
24+
+ "Example: " + COMMAND_WORD + " Roy";
2425

2526
public static final String MESSAGE_DELETE_PERSON_SUCCESS = "Deleted Person: %1$s";
2627

27-
private final Index targetIndex;
28+
private final Name targetName;
2829

29-
public DeleteCommand(Index targetIndex) {
30-
this.targetIndex = targetIndex;
30+
public DeleteCommand(Name targetName) {
31+
this.targetName = targetName;
3132
}
3233

3334
@Override
3435
public CommandResult execute(Model model) throws CommandException {
3536
requireNonNull(model);
36-
List<Person> lastShownList = model.getFilteredPersonList();
37+
boolean isValidContact = model.hasPersonName(targetName);
3738

38-
if (targetIndex.getZeroBased() >= lastShownList.size()) {
39-
throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
39+
if (!isValidContact) {
40+
throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED);
4041
}
4142

42-
Person personToDelete = lastShownList.get(targetIndex.getZeroBased());
43+
List<Person> filteredList = model.getFilteredPersonList().stream()
44+
.filter(person -> person.isSameName(targetName)).collect(Collectors.toList());
45+
Person personToDelete = filteredList.get(0);
4346
model.deletePerson(personToDelete);
4447
return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, personToDelete));
4548
}
@@ -48,6 +51,6 @@ public CommandResult execute(Model model) throws CommandException {
4851
public boolean equals(Object other) {
4952
return other == this // short circuit if same object
5053
|| (other instanceof DeleteCommand // instanceof handles nulls
51-
&& targetIndex.equals(((DeleteCommand) other).targetIndex)); // state check
54+
&& targetName.equals(((DeleteCommand) other).targetName)); // state check
5255
}
5356
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22

33
import static seedu.address.commons.core.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
44

5-
import seedu.address.commons.core.index.Index;
65
import seedu.address.logic.commands.DeleteCommand;
76
import seedu.address.logic.parser.exceptions.ParseException;
7+
import seedu.address.model.person.Name;
88

99
/**
1010
* Parses input arguments and creates a new DeleteCommand object
@@ -18,8 +18,8 @@ public class DeleteCommandParser implements Parser<DeleteCommand> {
1818
*/
1919
public DeleteCommand parse(String args) throws ParseException {
2020
try {
21-
Index index = ParserUtil.parseIndex(args);
22-
return new DeleteCommand(index);
21+
Name targetName = ParserUtil.parseName(args);
22+
return new DeleteCommand(targetName);
2323
} catch (ParseException pe) {
2424
throw new ParseException(
2525
String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE), pe);

‎src/main/java/seedu/address/model/AddressBook.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
import java.util.List;
66

77
import javafx.collections.ObservableList;
8+
import seedu.address.model.person.Name;
89
import seedu.address.model.person.Person;
910
import seedu.address.model.person.UniquePersonList;
1011

12+
1113
/**
1214
* Wraps all data at the address-book level
1315
* Duplicates are not allowed (by .isSamePerson comparison)
@@ -66,6 +68,14 @@ public boolean hasPerson(Person person) {
6668
return persons.contains(person);
6769
}
6870

71+
/**
72+
* Returns true if a person with the same name as {@code person} exists in the address book.
73+
*/
74+
public boolean hasPersonName(Name name) {
75+
requireNonNull(name);
76+
return persons.contains(name);
77+
}
78+
6979
/**
7080
* Adds a person to the address book.
7181
* The person must not already exist in the address book.

‎src/main/java/seedu/address/model/Model.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import javafx.collections.ObservableList;
77
import seedu.address.commons.core.GuiSettings;
8+
import seedu.address.model.person.Name;
89
import seedu.address.model.person.Person;
910

1011
/**
@@ -57,6 +58,11 @@ public interface Model {
5758
*/
5859
boolean hasPerson(Person person);
5960

61+
/**
62+
* Returns true if a person with the same name as {@code person} exists in the address book.
63+
*/
64+
boolean hasPersonName(Name name);
65+
6066
/**
6167
* Deletes the given person.
6268
* The person must exist in the address book.

‎src/main/java/seedu/address/model/ModelManager.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import javafx.collections.transformation.FilteredList;
1212
import seedu.address.commons.core.GuiSettings;
1313
import seedu.address.commons.core.LogsCenter;
14+
import seedu.address.model.person.Name;
1415
import seedu.address.model.person.Person;
1516

1617
/**
@@ -94,6 +95,12 @@ public boolean hasPerson(Person person) {
9495
return addressBook.hasPerson(person);
9596
}
9697

98+
@Override
99+
public boolean hasPersonName(Name name) {
100+
requireNonNull(name);
101+
return addressBook.hasPersonName(name);
102+
}
103+
97104
@Override
98105
public void deletePerson(Person target) {
99106
addressBook.removePerson(target);

‎src/main/java/seedu/address/model/person/Person.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,22 @@ public Set<Tag> getTags() {
5555
}
5656

5757
/**
58-
* Returns true if both persons of the same name have at least one other identity field that is the same.
59-
* This defines a weaker notion of equality between two persons.
58+
* Returns true if both persons have the same name.
6059
*/
6160
public boolean isSamePerson(Person otherPerson) {
6261
if (otherPerson == this) {
6362
return true;
6463
}
6564

6665
return otherPerson != null
67-
&& otherPerson.getName().equals(getName())
68-
&& (otherPerson.getPhone().equals(getPhone()) || otherPerson.getEmail().equals(getEmail()));
66+
&& otherPerson.getName().equals(getName());
67+
}
68+
69+
/**
70+
* Returns true if both persons have the same name.
71+
*/
72+
public boolean isSameName(Name otherName) {
73+
return name.equals(otherName);
6974
}
7075

7176
/**

‎src/main/java/seedu/address/model/person/UniquePersonList.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,14 @@ public boolean contains(Person toCheck) {
3636
return internalList.stream().anyMatch(toCheck::isSamePerson);
3737
}
3838

39+
/**
40+
* Returns true if the list contains a person with the same name.
41+
*/
42+
public boolean contains(Name toCheck) {
43+
requireNonNull(toCheck);
44+
return internalList.stream().anyMatch(person -> person.isSameName(toCheck));
45+
}
46+
3947
/**
4048
* Adds a person to the list.
4149
* The person must not already exist in the list.

‎src/test/java/seedu/address/logic/commands/AddCommandTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import seedu.address.model.Model;
2121
import seedu.address.model.ReadOnlyAddressBook;
2222
import seedu.address.model.ReadOnlyUserPrefs;
23+
import seedu.address.model.person.Name;
2324
import seedu.address.model.person.Person;
2425
import seedu.address.testutil.PersonBuilder;
2526

@@ -128,6 +129,11 @@ public boolean hasPerson(Person person) {
128129
throw new AssertionError("This method should not be called.");
129130
}
130131

132+
@Override
133+
public boolean hasPersonName(Name name) {
134+
throw new AssertionError("This method should not be called.");
135+
}
136+
131137
@Override
132138
public void deletePerson(Person target) {
133139
throw new AssertionError("This method should not be called.");

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

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
import static seedu.address.logic.commands.CommandTestUtil.showPersonAtIndex;
88
import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON;
99
import static seedu.address.testutil.TypicalIndexes.INDEX_SECOND_PERSON;
10+
import static seedu.address.testutil.TypicalPersons.ALICE;
11+
import static seedu.address.testutil.TypicalPersons.BOB;
1012
import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook;
1113

1214
import org.junit.jupiter.api.Test;
@@ -16,6 +18,7 @@
1618
import seedu.address.model.Model;
1719
import seedu.address.model.ModelManager;
1820
import seedu.address.model.UserPrefs;
21+
import seedu.address.model.person.Name;
1922
import seedu.address.model.person.Person;
2023

2124
/**
@@ -27,9 +30,9 @@ public class DeleteCommandTest {
2730
private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs());
2831

2932
@Test
30-
public void execute_validIndexUnfilteredList_success() {
33+
public void execute_validNameUnfilteredList_success() {
3134
Person personToDelete = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased());
32-
DeleteCommand deleteCommand = new DeleteCommand(INDEX_FIRST_PERSON);
35+
DeleteCommand deleteCommand = new DeleteCommand(personToDelete.getName());
3336

3437
String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_PERSON_SUCCESS, personToDelete);
3538

@@ -40,19 +43,18 @@ public void execute_validIndexUnfilteredList_success() {
4043
}
4144

4245
@Test
43-
public void execute_invalidIndexUnfilteredList_throwsCommandException() {
44-
Index outOfBoundIndex = Index.fromOneBased(model.getFilteredPersonList().size() + 1);
45-
DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex);
46+
public void execute_invalidNameUnfilteredList_throwsCommandException() {
47+
DeleteCommand deleteCommand = new DeleteCommand(new Name("123"));
4648

47-
assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
49+
assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED);
4850
}
4951

5052
@Test
51-
public void execute_validIndexFilteredList_success() {
53+
public void execute_validNameFilteredList_success() {
5254
showPersonAtIndex(model, INDEX_FIRST_PERSON);
5355

5456
Person personToDelete = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased());
55-
DeleteCommand deleteCommand = new DeleteCommand(INDEX_FIRST_PERSON);
57+
DeleteCommand deleteCommand = new DeleteCommand(personToDelete.getName());
5658

5759
String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_PERSON_SUCCESS, personToDelete);
5860

@@ -64,28 +66,28 @@ public void execute_validIndexFilteredList_success() {
6466
}
6567

6668
@Test
67-
public void execute_invalidIndexFilteredList_throwsCommandException() {
69+
public void execute_invalidNameFilteredList_throwsCommandException() {
6870
showPersonAtIndex(model, INDEX_FIRST_PERSON);
6971

7072
Index outOfBoundIndex = INDEX_SECOND_PERSON;
7173
// ensures that outOfBoundIndex is still in bounds of address book list
7274
assertTrue(outOfBoundIndex.getZeroBased() < model.getAddressBook().getPersonList().size());
7375

74-
DeleteCommand deleteCommand = new DeleteCommand(outOfBoundIndex);
76+
DeleteCommand deleteCommand = new DeleteCommand(new Name("123"));
7577

76-
assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
78+
assertCommandFailure(deleteCommand, model, Messages.MESSAGE_INVALID_PERSON_DISPLAYED);
7779
}
7880

7981
@Test
8082
public void equals() {
81-
DeleteCommand deleteFirstCommand = new DeleteCommand(INDEX_FIRST_PERSON);
82-
DeleteCommand deleteSecondCommand = new DeleteCommand(INDEX_SECOND_PERSON);
83+
DeleteCommand deleteFirstCommand = new DeleteCommand(ALICE.getName());
84+
DeleteCommand deleteSecondCommand = new DeleteCommand(BOB.getName());
8385

8486
// same object -> returns true
8587
assertTrue(deleteFirstCommand.equals(deleteFirstCommand));
8688

8789
// same values -> returns true
88-
DeleteCommand deleteFirstCommandCopy = new DeleteCommand(INDEX_FIRST_PERSON);
90+
DeleteCommand deleteFirstCommandCopy = new DeleteCommand(ALICE.getName());
8991
assertTrue(deleteFirstCommand.equals(deleteFirstCommandCopy));
9092

9193
// different types -> returns false

0 commit comments

Comments
 (0)