Skip to content

Commit

Permalink
AddressBook: remove master tag list
Browse files Browse the repository at this point in the history
We keep track of a master tag list which holds all the tags in
AddressBook.

As the master tag list was deemed unnecessary and may be too complex
for new developers to grasp, as discussed in
se-edu/addressbook-level4#753 and se-edu/addressbook-level4#794,
the master tag list has already been removed in addressbook-level4
in se-edu/addressbook-level4#824. As such, the master tag list
should also be removed here for consistency.

Let's remove the master tag list in AddressBook.
  • Loading branch information
yamidark committed Aug 6, 2018
1 parent 49cdf25 commit ee3077e
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 74 deletions.
55 changes: 9 additions & 46 deletions src/seedu/addressbook/data/AddressBook.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package seedu.addressbook.data;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

import seedu.addressbook.data.person.Person;
Expand All @@ -15,72 +13,34 @@

/**
* Represents the entire address book. Contains the data of the address book.
*
* Guarantees:
* - Every tag found in every person will also be found in the tag list.
* - The tags in each person point to tag objects in the master list. (== equality)
*/
public class AddressBook {

private final UniquePersonList allPersons;
private final UniqueTagList allTags; // can contain tags not attached to any person

/**
* Creates an empty address book.
*/
public AddressBook() {
allPersons = new UniquePersonList();
allTags = new UniqueTagList();
}

/**
* Constructs an address book with the given data.
* Also updates the tag list with any missing tags found in any person.
*
* @param persons external changes to this will not affect this address book
* @param tags external changes to this will not affect this address book
*/
public AddressBook(UniquePersonList persons, UniqueTagList tags) {
public AddressBook(UniquePersonList persons) {
this.allPersons = new UniquePersonList(persons);
this.allTags = new UniqueTagList(tags);
for (Person p : allPersons) {
syncTagsWithMasterList(p);
}
}

/**
* Ensures that every tag in this person:
* - exists in the master list {@link #allTags}
* - points to a Tag object in the master list
*/
private void syncTagsWithMasterList(Person person) {
final UniqueTagList personTags = person.getTags();
allTags.mergeFrom(personTags);

// Create map with values = tag object references in the master list
final Map<Tag, Tag> masterTagObjects = new HashMap<>();
for (Tag tag : allTags) {
masterTagObjects.put(tag, tag);
}

// Rebuild the list of person tags using references from the master list
final Set<Tag> commonTagReferences = new HashSet<>();
for (Tag tag : personTags) {
commonTagReferences.add(masterTagObjects.get(tag));
}
person.setTags(new UniqueTagList(commonTagReferences));
}

/**
* Adds a person to the address book.
* Also checks the new person's tags and updates {@link #allTags} with any new tags found,
* and updates the Tag objects in the person to point to those in {@link #allTags}.
*
* @throws DuplicatePersonException if an equivalent person already exists.
*/
public void addPerson(Person toAdd) throws DuplicatePersonException {
allPersons.add(toAdd);
syncTagsWithMasterList(toAdd);
}

/**
Expand All @@ -104,7 +64,6 @@ public void removePerson(ReadOnlyPerson toRemove) throws PersonNotFoundException
*/
public void clear() {
allPersons.clear();
allTags.clear();
}

/**
Expand All @@ -115,17 +74,21 @@ public UniquePersonList getAllPersons() {
}

/**
* Returns a new UniqueTagList of all tags in the address book at the time of the call.
* Returns a new UniqueTagList of all tags in all persons in the address book at the time of the call.
*/
public UniqueTagList getAllTags() {
return new UniqueTagList(allTags);
Set<Tag> tagSet = new HashSet<>();
for (Person person : allPersons) {
tagSet.addAll(person.getTags().toSet());
}

return new UniqueTagList(tagSet);
}

@Override
public boolean equals(Object other) {
return other == this // short circuit if same object
|| (other instanceof AddressBook // instanceof handles nulls
&& this.allPersons.equals(((AddressBook) other).allPersons)
&& this.allTags.equals(((AddressBook) other).allTags));
&& this.allPersons.equals(((AddressBook) other).allPersons));
}
}
7 changes: 1 addition & 6 deletions src/seedu/addressbook/storage/jaxb/AdaptedAddressBook.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import seedu.addressbook.data.AddressBook;
import seedu.addressbook.data.exception.IllegalValueException;
import seedu.addressbook.data.tag.Tag;
import seedu.addressbook.data.tag.UniqueTagList;
import seedu.addressbook.data.person.Person;
import seedu.addressbook.data.person.ReadOnlyPerson;
import seedu.addressbook.data.person.UniquePersonList;
Expand Down Expand Up @@ -74,14 +73,10 @@ public boolean isAnyRequiredFieldMissing() {
* @throws IllegalValueException if there were any data constraints violated in the adapted person
*/
public AddressBook toModelType() throws IllegalValueException {
final List<Tag> tagList = new ArrayList<>();
final List<Person> personList = new ArrayList<>();
for (AdaptedTag tag : tags) {
tagList.add(tag.toModelType());
}
for (AdaptedPerson person : persons) {
personList.add(person.toModelType());
}
return new AddressBook(new UniquePersonList(personList), new UniqueTagList(tagList));
return new AddressBook(new UniquePersonList(personList));
}
}
23 changes: 2 additions & 21 deletions test/java/seedu/addressbook/data/AddressBookTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ public void setUp() throws Exception {
new UniqueTagList(tagEconomist, tagPrizeWinner));

emptyAddressBook = new AddressBook();
defaultAddressBook = new AddressBook(new UniquePersonList(aliceBetsy, bobChaplin),
new UniqueTagList(tagMathematician, tagScientist));
defaultAddressBook = new AddressBook(new UniquePersonList(aliceBetsy, bobChaplin));
}

@Rule
Expand All @@ -81,12 +80,8 @@ public void addPerson_emptyAddressBook() throws Exception {
emptyAddressBook.addPerson(bobChaplin);
emptyAddressBook.addPerson(charlieDouglas);

UniqueTagList expectedTagList = new UniqueTagList(tagMathematician, tagScientist);
assertTrue(isIdentical(expectedTagList, emptyAddressBook.getAllTags()));

assertTrue(isTagObjectInAddressBookList(tagMathematician, emptyAddressBook));
assertTrue(isTagObjectInAddressBookList(tagScientist, emptyAddressBook));

}

@Test
Expand All @@ -104,19 +99,6 @@ public void addPerson_personAlreadyInList_throwsDuplicatePersonException() throw
defaultAddressBook.addPerson(aliceBetsy);
}

@Test
public void addPerson_personAlreadyInListButHasTagNotInList_tagNotAdded() throws Exception {
aliceBetsy.setTags(new UniqueTagList(tagMathematician, tagPrizeWinner));

try {
defaultAddressBook.addPerson(aliceBetsy);
} catch (DuplicatePersonException e) {
// ignore expected exception
}

assertFalse(isTagObjectInAddressBookList(tagPrizeWinner, defaultAddressBook));
}

@Test
public void containsPerson() throws Exception {
UniquePersonList personsWhoShouldBeIn = new UniquePersonList(aliceBetsy, bobChaplin);
Expand Down Expand Up @@ -173,9 +155,8 @@ public void getAllPersons() throws Exception {
@Test
public void getAllTags() throws Exception {
UniqueTagList allTags = defaultAddressBook.getAllTags();
UniqueTagList tagsToCheck = new UniqueTagList(tagMathematician, tagScientist);

assertTrue(isIdentical(allTags, tagsToCheck));
assertTrue(allTags.contains(tagMathematician));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion test/java/seedu/addressbook/util/TestUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public static List<ReadOnlyPerson> createList(Person...persons) {
* of Persons and Tags. The Persons and Tags are not cloned.
*/
public static AddressBook clone(AddressBook addressBook) {
return new AddressBook(addressBook.getAllPersons(), addressBook.getAllTags());
return new AddressBook(addressBook.getAllPersons());
}

/**
Expand Down

0 comments on commit ee3077e

Please sign in to comment.