Skip to content

Commit

Permalink
Remove ReadOnlyPerson
Browse files Browse the repository at this point in the history
ReadOnlyPerson provides a read-only immutable interface for Person.

Person is already immutable. This removes the need for ReadOnlyPerson.

Let's replace all instances of ReadOnlyPerson with Person.

Mechanical changes:
1. Delete ReadOnlyPerson. Ensure that all
"import seedu.address.model.person.ReadOnlyPerson;" statements are
deleted.
2. Use the replace all functionality to replace
"? extends ReadOnlyPerson" with "Person" for all text files.
3. Use the replace all functionality to replace
"ReadOnlyPerson" with "Person" for all text files.

At this point, the code base will not compile as the
"import seedu.address.model.person.Person;" statement is missing in
some files. You will have to manually add this line to those files,
using your IDE's tooling if applicable. On Eclipse, you can press
Ctrl + Shift + O to do so.

Furthermore, in AddressBook#updatePerson(Person, Person), the code will
not compile because a variable is re-defined. This will also need to be
manually fixed as well.

Finally, rewrap some lines which have become too short as a result of
the find-and-replace in steps 2 and 3.

This commit was performed using IntelliJ.
  • Loading branch information
Zhiyuan-Amos committed Jan 17, 2018
1 parent c4758e1 commit 345d05f
Show file tree
Hide file tree
Showing 44 changed files with 167 additions and 202 deletions.
16 changes: 8 additions & 8 deletions docs/DeveloperGuide.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ The `Model`,

* stores a `UserPref` object that represents the user's preferences.
* stores the Address Book data.
* exposes an unmodifiable `ObservableList<ReadOnlyPerson>` that can be 'observed' e.g. the UI can be bound to this list so that the UI automatically updates when the data in the list change.
* exposes an unmodifiable `ObservableList<Person>` that can be 'observed' e.g. the UI can be bound to this list so that the UI automatically updates when the data in the list change.
* does not depend on any of the other three components.

[[Design-Storage]]
Expand Down Expand Up @@ -561,7 +561,7 @@ image::getting-started-ui-tag-after.png[width="300"]
+
****
* Hints
** The tag labels are created inside link:{repoURL}/src/main/java/seedu/address/ui/PersonCard.java[`PersonCard#initTags(ReadOnlyPerson)`] (`new Label(tag.tagName)`). https://docs.oracle.com/javase/8/javafx/api/javafx/scene/control/Label.html[JavaFX's `Label` class] allows you to modify the style of each Label, such as changing its color.
** The tag labels are created inside link:{repoURL}/src/main/java/seedu/address/ui/PersonCard.java[`PersonCard#initTags(Person)`] (`new Label(tag.tagName)`). https://docs.oracle.com/javase/8/javafx/api/javafx/scene/control/Label.html[JavaFX's `Label` class] allows you to modify the style of each Label, such as changing its color.
** Use the .css attribute `-fx-background-color` to add a color.
* Solution
** See this https://github.com/se-edu/addressbook-level4/pull/592/files[PR] for the full solution.
Expand Down Expand Up @@ -683,7 +683,7 @@ Let's add a placeholder on all our link:{repoURL}/src/main/java/seedu/address/ui
. Modify link:{repoURL}/src/test/java/guitests/guihandles/PersonCardHandle.java[`PersonCardHandle`] so that future tests can read the contents of the remark label.

===== [Step 4] Model: Add `Remark` class
We have to properly encapsulate the remark in our link:{repoURL}/src/main/java/seedu/address/model/person/ReadOnlyPerson.java[`ReadOnlyPerson`] class. Instead of just using a `String`, let's follow the conventional class structure that the codebase already uses by adding a `Remark` class.
We have to properly encapsulate the remark in our link:{repoURL}/src/main/java/seedu/address/model/person/Person.java[`Person`] class. Instead of just using a `String`, let's follow the conventional class structure that the codebase already uses by adding a `Remark` class.

**Main:**

Expand All @@ -694,12 +694,12 @@ We have to properly encapsulate the remark in our link:{repoURL}/src/main/java/s

. Add test for `Remark`, to test the `Remark#equals()` method.

===== [Step 5] Model: Modify `ReadOnlyPerson` to support a `Remark` field
Now we have the `Remark` class, we need to actually use it inside link:{repoURL}/src/main/java/seedu/address/model/person/ReadOnlyPerson.java[`ReadOnlyPerson`].
===== [Step 5] Model: Modify `Person` to support a `Remark` field
Now we have the `Remark` class, we need to actually use it inside link:{repoURL}/src/main/java/seedu/address/model/person/Person.java[`Person`].

**Main:**

. Add three methods `setRemark(Remark)`, `getRemark()` and `remarkProperty()`. Be sure to implement these newly created methods in link:{repoURL}/src/main/java/seedu/address/model/person/ReadOnlyPerson.java[`Person`], which implements the link:{repoURL}/src/main/java/seedu/address/model/person/ReadOnlyPerson.java[`ReadOnlyPerson`] interface.
. Add three methods `setRemark(Remark)`, `getRemark()` and `remarkProperty()`. Be sure to implement these newly created methods in link:{repoURL}/src/main/java/seedu/address/model/person/Person.java[`Person`], which implements the link:{repoURL}/src/main/java/seedu/address/model/person/Person.java[`Person`] interface.
. You may assume that the user will not be able to use the `add` and `edit` commands to modify the remarks field (i.e. the person will be created without a remark).
. Modify link:{repoURL}/src/main/java/seedu/address/model/util/SampleDataUtil.java/[`SampleDataUtil`] to add remarks for the sample data (delete your `addressBook.xml` so that the application will load the sample data when you launch it.)

Expand All @@ -709,7 +709,7 @@ We now have `Remark` s for `Person` s, but they will be gone when we exit the ap
**Main:**

. Add a new Xml field for `Remark`.
. Be sure to modify the logic of the constructor and `toModelType()`, which handles the conversion to/from link:{repoURL}/src/main/java/seedu/address/model/person/ReadOnlyPerson.java[`ReadOnlyPerson`].
. Be sure to modify the logic of the constructor and `toModelType()`, which handles the conversion to/from link:{repoURL}/src/main/java/seedu/address/model/person/Person.java[`Person`].

**Tests:**

Expand All @@ -725,7 +725,7 @@ Our remark label in link:{repoURL}/src/main/java/seedu/address/ui/PersonCard.jav
**Tests:**

. Modify link:{repoURL}/src/test/java/seedu/address/ui/testutil/GuiTestAssert.java[`GuiTestAssert#assertCardDisplaysPerson(...)`] so that it will compare the remark label.
. In link:{repoURL}/src/test/java/seedu/address/ui/PersonCardTest.java[`PersonCardTest`], call `personWithTags.setRemark(ALICE.getRemark())` to test that changes in the link:{repoURL}/src/main/java/seedu/address/model/person/ReadOnlyPerson.java[`Person`] 's remark correctly updates the corresponding link:{repoURL}/src/main/java/seedu/address/ui/PersonCard.java[`PersonCard`].
. In link:{repoURL}/src/test/java/seedu/address/ui/PersonCardTest.java[`PersonCardTest`], call `personWithTags.setRemark(ALICE.getRemark())` to test that changes in the link:{repoURL}/src/main/java/seedu/address/model/person/Person.java[`Person`] 's remark correctly updates the corresponding link:{repoURL}/src/main/java/seedu/address/ui/PersonCard.java[`PersonCard`].

===== [Step 8] Logic: Implement `RemarkCommand#execute()` logic
We now have everything set up... but we still can't modify the remarks. Let's finish it up by adding in actual logic for our `remark` command.
Expand Down
Binary file modified docs/diagrams/ModelComponentClassDiagram.pptx
Binary file not shown.
Binary file modified docs/images/ModelClassDiagram.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions src/main/java/seedu/address/logic/Logic.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import seedu.address.logic.commands.CommandResult;
import seedu.address.logic.commands.exceptions.CommandException;
import seedu.address.logic.parser.exceptions.ParseException;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.model.person.Person;

/**
* API of the Logic component
Expand All @@ -20,7 +20,7 @@ public interface Logic {
CommandResult execute(String commandText) throws CommandException, ParseException;

/** Returns an unmodifiable view of the filtered list of persons */
ObservableList<ReadOnlyPerson> getFilteredPersonList();
ObservableList<Person> getFilteredPersonList();

/** Returns the list of input entered by the user, encapsulated in a {@code ListElementPointer} object */
ListElementPointer getHistorySnapshot();
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/seedu/address/logic/LogicManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import seedu.address.logic.parser.AddressBookParser;
import seedu.address.logic.parser.exceptions.ParseException;
import seedu.address.model.Model;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.model.person.Person;

/**
* The main LogicManager of the app.
Expand Down Expand Up @@ -46,7 +46,7 @@ public CommandResult execute(String commandText) throws CommandException, ParseE
}

@Override
public ObservableList<ReadOnlyPerson> getFilteredPersonList() {
public ObservableList<Person> getFilteredPersonList() {
return model.getFilteredPersonList();
}

Expand Down
5 changes: 2 additions & 3 deletions src/main/java/seedu/address/logic/commands/AddCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import seedu.address.logic.commands.exceptions.CommandException;
import seedu.address.model.person.Person;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.model.person.exceptions.DuplicatePersonException;

/**
Expand Down Expand Up @@ -40,9 +39,9 @@ public class AddCommand extends UndoableCommand {
private final Person toAdd;

/**
* Creates an AddCommand to add the specified {@code ReadOnlyPerson}
* Creates an AddCommand to add the specified {@code Person}
*/
public AddCommand(ReadOnlyPerson person) {
public AddCommand(Person person) {
toAdd = new Person(person);
}

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/seedu/address/logic/commands/DeleteCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import seedu.address.commons.core.Messages;
import seedu.address.commons.core.index.Index;
import seedu.address.logic.commands.exceptions.CommandException;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.model.person.Person;
import seedu.address.model.person.exceptions.PersonNotFoundException;

/**
Expand All @@ -32,13 +32,13 @@ public DeleteCommand(Index targetIndex) {
@Override
public CommandResult executeUndoableCommand() throws CommandException {

List<ReadOnlyPerson> lastShownList = model.getFilteredPersonList();
List<Person> lastShownList = model.getFilteredPersonList();

if (targetIndex.getZeroBased() >= lastShownList.size()) {
throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
}

ReadOnlyPerson personToDelete = lastShownList.get(targetIndex.getZeroBased());
Person personToDelete = lastShownList.get(targetIndex.getZeroBased());

try {
model.deletePerson(personToDelete);
Expand Down
8 changes: 3 additions & 5 deletions src/main/java/seedu/address/logic/commands/EditCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import seedu.address.model.person.Name;
import seedu.address.model.person.Person;
import seedu.address.model.person.Phone;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.model.person.exceptions.DuplicatePersonException;
import seedu.address.model.person.exceptions.PersonNotFoundException;
import seedu.address.model.tag.Tag;
Expand Down Expand Up @@ -67,13 +66,13 @@ public EditCommand(Index index, EditPersonDescriptor editPersonDescriptor) {

@Override
public CommandResult executeUndoableCommand() throws CommandException {
List<ReadOnlyPerson> lastShownList = model.getFilteredPersonList();
List<Person> lastShownList = model.getFilteredPersonList();

if (index.getZeroBased() >= lastShownList.size()) {
throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
}

ReadOnlyPerson personToEdit = lastShownList.get(index.getZeroBased());
Person personToEdit = lastShownList.get(index.getZeroBased());
Person editedPerson = createEditedPerson(personToEdit, editPersonDescriptor);

try {
Expand All @@ -91,8 +90,7 @@ public CommandResult executeUndoableCommand() throws CommandException {
* Creates and returns a {@code Person} with the details of {@code personToEdit}
* edited with {@code editPersonDescriptor}.
*/
private static Person createEditedPerson(ReadOnlyPerson personToEdit,
EditPersonDescriptor editPersonDescriptor) {
private static Person createEditedPerson(Person personToEdit, EditPersonDescriptor editPersonDescriptor) {
assert personToEdit != null;

Name updatedName = editPersonDescriptor.getName().orElse(personToEdit.getName());
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/seedu/address/logic/commands/SelectCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import seedu.address.commons.core.index.Index;
import seedu.address.commons.events.ui.JumpToListRequestEvent;
import seedu.address.logic.commands.exceptions.CommandException;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.model.person.Person;

/**
* Selects a person identified using it's last displayed index from the address book.
Expand All @@ -32,7 +32,7 @@ public SelectCommand(Index targetIndex) {
@Override
public CommandResult execute() throws CommandException {

List<ReadOnlyPerson> lastShownList = model.getFilteredPersonList();
List<Person> lastShownList = model.getFilteredPersonList();

if (targetIndex.getZeroBased() >= lastShownList.size()) {
throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import seedu.address.model.person.Name;
import seedu.address.model.person.Person;
import seedu.address.model.person.Phone;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.model.tag.Tag;

/**
Expand Down Expand Up @@ -47,7 +46,7 @@ public AddCommand parse(String args) throws ParseException {
Address address = ParserUtil.parseAddress(argMultimap.getValue(PREFIX_ADDRESS)).get();
Set<Tag> tagList = ParserUtil.parseTags(argMultimap.getAllValues(PREFIX_TAG));

ReadOnlyPerson person = new Person(name, phone, email, address, tagList);
Person person = new Person(name, phone, email, address, tagList);

return new AddCommand(person);
} catch (IllegalValueException ive) {
Expand Down
25 changes: 12 additions & 13 deletions src/main/java/seedu/address/model/AddressBook.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

import javafx.collections.ObservableList;
import seedu.address.model.person.Person;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.model.person.UniquePersonList;
import seedu.address.model.person.exceptions.DuplicatePersonException;
import seedu.address.model.person.exceptions.PersonNotFoundException;
Expand Down Expand Up @@ -52,7 +51,7 @@ public AddressBook(ReadOnlyAddressBook toBeCopied) {

//// list overwrite operations

public void setPersons(List<? extends ReadOnlyPerson> persons) throws DuplicatePersonException {
public void setPersons(List<Person> persons) throws DuplicatePersonException {
this.persons.setPersons(persons);
}

Expand Down Expand Up @@ -86,7 +85,7 @@ public void resetData(ReadOnlyAddressBook newData) {
*
* @throws DuplicatePersonException if an equivalent person already exists.
*/
public void addPerson(ReadOnlyPerson p) throws DuplicatePersonException {
public void addPerson(Person p) throws DuplicatePersonException {
Person person = syncWithMasterTagList(p);
// TODO: the tags master list will be updated even though the below line fails.
// This can cause the tags master list to have additional tags that are not tagged to any person
Expand All @@ -95,32 +94,32 @@ public void addPerson(ReadOnlyPerson p) throws DuplicatePersonException {
}

/**
* Replaces the given person {@code target} in the list with {@code editedReadOnlyPerson}.
* {@code AddressBook}'s tag list will be updated with the tags of {@code editedReadOnlyPerson}.
* Replaces the given person {@code target} in the list with {@code editedPerson}.
* {@code AddressBook}'s tag list will be updated with the tags of {@code editedPerson}.
*
* @throws DuplicatePersonException if updating the person's details causes the person to be equivalent to
* another existing person in the list.
* @throws PersonNotFoundException if {@code target} could not be found in the list.
*
* @see #syncWithMasterTagList(ReadOnlyPerson)
* @see #syncWithMasterTagList(Person)
*/
public void updatePerson(ReadOnlyPerson target, ReadOnlyPerson editedReadOnlyPerson)
public void updatePerson(Person target, Person editedPerson)
throws DuplicatePersonException, PersonNotFoundException {
requireNonNull(editedReadOnlyPerson);
requireNonNull(editedPerson);

Person editedPerson = syncWithMasterTagList(editedReadOnlyPerson);
Person syncedEditedPerson = syncWithMasterTagList(editedPerson);
// TODO: the tags master list will be updated even though the below line fails.
// This can cause the tags master list to have additional tags that are not tagged to any person
// in the person list.
persons.setPerson(target, editedPerson);
persons.setPerson(target, syncedEditedPerson);
}

/**
* Updates the master tag list to include tags in {@code person} that are not in the list.
* @return a copy of this {@code person} such that every tag in this person points to a Tag object in the master
* list.
*/
private Person syncWithMasterTagList(ReadOnlyPerson person) {
private Person syncWithMasterTagList(Person person) {
final UniqueTagList personTags = new UniqueTagList(person.getTags());
tags.mergeFrom(personTags);

Expand All @@ -140,7 +139,7 @@ private Person syncWithMasterTagList(ReadOnlyPerson person) {
* Removes {@code key} from this {@code AddressBook}.
* @throws PersonNotFoundException if the {@code key} is not in this {@code AddressBook}.
*/
public boolean removePerson(ReadOnlyPerson key) throws PersonNotFoundException {
public boolean removePerson(Person key) throws PersonNotFoundException {
if (persons.remove(key)) {
return true;
} else {
Expand All @@ -163,7 +162,7 @@ public String toString() {
}

@Override
public ObservableList<ReadOnlyPerson> getPersonList() {
public ObservableList<Person> getPersonList() {
return persons.asObservableList();
}

Expand Down
14 changes: 7 additions & 7 deletions src/main/java/seedu/address/model/Model.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import java.util.function.Predicate;

import javafx.collections.ObservableList;
import seedu.address.model.person.ReadOnlyPerson;
import seedu.address.model.person.Person;
import seedu.address.model.person.exceptions.DuplicatePersonException;
import seedu.address.model.person.exceptions.PersonNotFoundException;

Expand All @@ -12,7 +12,7 @@
*/
public interface Model {
/** {@code Predicate} that always evaluate to true */
Predicate<ReadOnlyPerson> PREDICATE_SHOW_ALL_PERSONS = unused -> true;
Predicate<Person> PREDICATE_SHOW_ALL_PERSONS = unused -> true;

/** Clears existing backing model and replaces with the provided new data. */
void resetData(ReadOnlyAddressBook newData);
Expand All @@ -21,10 +21,10 @@ public interface Model {
ReadOnlyAddressBook getAddressBook();

/** Deletes the given person. */
void deletePerson(ReadOnlyPerson target) throws PersonNotFoundException;
void deletePerson(Person target) throws PersonNotFoundException;

/** Adds the given person */
void addPerson(ReadOnlyPerson person) throws DuplicatePersonException;
void addPerson(Person person) throws DuplicatePersonException;

/**
* Replaces the given person {@code target} with {@code editedPerson}.
Expand All @@ -33,16 +33,16 @@ public interface Model {
* another existing person in the list.
* @throws PersonNotFoundException if {@code target} could not be found in the list.
*/
void updatePerson(ReadOnlyPerson target, ReadOnlyPerson editedPerson)
void updatePerson(Person target, Person editedPerson)
throws DuplicatePersonException, PersonNotFoundException;

/** Returns an unmodifiable view of the filtered person list */
ObservableList<ReadOnlyPerson> getFilteredPersonList();
ObservableList<Person> getFilteredPersonList();

/**
* Updates the filter of the filtered person list to filter by the given {@code predicate}.
* @throws NullPointerException if {@code predicate} is null.
*/
void updateFilteredPersonList(Predicate<ReadOnlyPerson> predicate);
void updateFilteredPersonList(Predicate<Person> predicate);

}
Loading

0 comments on commit 345d05f

Please sign in to comment.