Skip to content

Commit

Permalink
AddressBook: remove unused containsTag()
Browse files Browse the repository at this point in the history
AddressBook#containsTag() checks if the master list of Tags contains a
Tag with the same value as the given Tag.

AddressBook#containsTag() is not used in the production code. While it
is used in test code, its use can be replaced by
AddressBookTest#isTagObjectInAddressBookList().

Let's remove AddressBook#containsTag().

Notes:
* Why not keep AddressBook#containsTag() and remove
AddressBookTest#isTagObjectInAddressBookList() instead? The former does
a value equality test while the latter goes further and does a reference
equality test. The test code requires a reference equality test to
verify Person objects refer to Tag objects in the common tag list
instead of keeping its own copies of Tag objects.
* Why not change AddressBook#containsTag() to use reference equality and
use that in tests? Doing so will make AddressBook#containsTag()'s
semantics inconsistent with the rest of our API.
  • Loading branch information
PierceAndy committed Feb 15, 2017
1 parent 4e3fac9 commit a6cc952
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 32 deletions.
7 changes: 0 additions & 7 deletions src/seedu/addressbook/data/AddressBook.java
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,6 @@ public boolean containsPerson(ReadOnlyPerson key) {
return allPersons.contains(key);
}

/**
* Returns true if an equivalent person exists in the address book.
*/
public boolean containsTag(Tag key) {
return allTags.contains(key);
}

/**
* Removes the equivalent person from the address book.
*
Expand Down
28 changes: 3 additions & 25 deletions test/java/seedu/addressbook/data/AddressBookTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,9 @@ public void addPerson_emptyAddressBook() throws Exception {

@Test
public void addPerson_someTagsNotInTagList() throws Exception {
assertFalse(defaultAddressBook.containsTag(tagEconomist));
assertFalse(defaultAddressBook.containsTag(tagPrizeWinner));
assertFalse(isTagObjectInAddressBookList(tagEconomist, defaultAddressBook));
assertFalse(isTagObjectInAddressBookList(tagPrizeWinner, defaultAddressBook));
defaultAddressBook.addPerson(davidElliot);
assertTrue(defaultAddressBook.containsTag(tagEconomist));
assertTrue(defaultAddressBook.containsTag(tagPrizeWinner));

assertTrue(isTagObjectInAddressBookList(tagEconomist, defaultAddressBook));
assertTrue(isTagObjectInAddressBookList(tagPrizeWinner, defaultAddressBook));
}
Expand All @@ -117,7 +114,7 @@ public void addPerson_personAlreadyInListButHasTagNotInList_tagNotAdded() throws
// ignore expected exception
}

assertFalse(defaultAddressBook.containsTag(tagPrizeWinner));
assertFalse(isTagObjectInAddressBookList(tagPrizeWinner, defaultAddressBook));
}

@Test
Expand All @@ -139,25 +136,6 @@ public void containsPerson() throws Exception {
}
}

@Test
public void containsTag() throws Exception {
UniqueTagList tagsWhichShouldBeIn = new UniqueTagList(tagMathematician, tagScientist);
UniqueTagList tagsWHichShouldNotBeIn = new UniqueTagList(tagEconomist, tagPrizeWinner);

for (Tag tagWhichShouldBeIn : tagsWhichShouldBeIn) {
assertTrue(defaultAddressBook.containsTag(tagWhichShouldBeIn));
}
for (Tag tagWhichShouldNotBeIn : tagsWHichShouldNotBeIn) {
assertFalse(defaultAddressBook.containsTag(tagWhichShouldNotBeIn));
}

UniqueTagList allTags = new UniqueTagList(tagPrizeWinner, tagScientist, tagMathematician, tagEconomist);

for (Tag tag : allTags) {
assertFalse(emptyAddressBook.containsTag(tag));
}
}

@Test
public void removePerson_personExists_removesNormally() throws Exception {
int numberOfPersonsBeforeRemoval = getSize(defaultAddressBook.getAllPersons());
Expand Down

0 comments on commit a6cc952

Please sign in to comment.