Refactor sorting of person list

Bug detected with sorting when other commands happen, such as sorting
then adding policy. Bug detected with not updating client view after
sorting.

Refactoring sorting will allow filteredPersons to revert to being
immutable and final, while allowing combinations of both
sorting and filtering effects.

Let's
* Refactor sort, by storing the comparator in model and allowing this
to be set.
* Layer of sorting logic when filtered list is obtained, so that
filtered list can remain immutable.
* Abstract setting the display of client to the first person in the
sorted and filtered list, since its used in find, sort and list

src/main/java/seedu/address/logic/LogicManager.java

@@ -69,7 +69,7 @@ public ReadOnlyAddressBook getAddressBook() {
 
     @Override
     public ObservableList<Person> getFilteredPersonList() {
-        return model.getFilteredPersonList();
+        return model.getSortedFilteredPersonList();
     }
 
     @Override

src/main/java/seedu/address/logic/commands/AddPolicyCommand.java

@@ -3,6 +3,7 @@
 import static java.util.Objects.requireNonNull;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_POLICYID;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_POLICYNAME;
+import static seedu.address.model.Model.COMPARATOR_SHOW_ORIGINAL_ORDER;
 import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 
 import java.util.List;
@@ -52,7 +53,7 @@ public AddPolicyCommand(Index index, Policy policy) {
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
 
-        List<Person> lastShownList = model.getFilteredPersonList();
+        List<Person> lastShownList = model.getSortedFilteredPersonList();
 
         if (index.getZeroBased() >= lastShownList.size()) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
@@ -74,6 +75,7 @@ public CommandResult execute(Model model) throws CommandException {
 
         model.setPerson(personToAddPolicy, policyAddedPerson);
         model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        model.updateSortPersonComparator(COMPARATOR_SHOW_ORIGINAL_ORDER);
         model.setDisplayClient(policyAddedPerson);
 
         return new CommandResult(String.format(MESSAGE_SUCCESS, personToAddPolicy.getName()));

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

@@ -34,7 +34,7 @@ public DeleteCommand(Index targetIndex) {
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        List<Person> lastShownList = model.getFilteredPersonList();
+        List<Person> lastShownList = model.getSortedFilteredPersonList();
 
         if (targetIndex.getZeroBased() >= lastShownList.size()) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);

src/main/java/seedu/address/logic/commands/DeletePolicyCommand.java

@@ -2,6 +2,7 @@
 
 import static java.util.Objects.requireNonNull;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_POLICYID;
+import static seedu.address.model.Model.COMPARATOR_SHOW_ORIGINAL_ORDER;
 import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 
 import java.util.List;
@@ -48,7 +49,7 @@ public DeletePolicyCommand(Index index, String policyId) {
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
 
-        List<Person> lastShownList = model.getFilteredPersonList();
+        List<Person> lastShownList = model.getSortedFilteredPersonList();
 
         if (index.getZeroBased() >= lastShownList.size()) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
@@ -70,6 +71,7 @@ public CommandResult execute(Model model) throws CommandException {
 
         model.setPerson(personToDeletePolicy, policyDeletedPerson);
         model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        model.updateSortPersonComparator(COMPARATOR_SHOW_ORIGINAL_ORDER);
         model.setDisplayClient(policyDeletedPerson);
 
         return new CommandResult(String.format(MESSAGE_SUCCESS, policyId, policyDeletedPerson.getName()));

src/main/java/seedu/address/logic/commands/EditCommand.java

@@ -8,6 +8,7 @@
 import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_PRIORITY;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;
+import static seedu.address.model.Model.COMPARATOR_SHOW_ORIGINAL_ORDER;
 import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 
 import java.util.Collections;
@@ -80,7 +81,7 @@ public EditCommand(Index index, EditPersonDescriptor editPersonDescriptor) {
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        List<Person> lastShownList = model.getFilteredPersonList();
+        List<Person> lastShownList = model.getSortedFilteredPersonList();
 
         if (index.getZeroBased() >= lastShownList.size()) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
@@ -95,6 +96,7 @@ public CommandResult execute(Model model) throws CommandException {
 
         model.setPerson(personToEdit, editedPerson);
         model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        model.updateSortPersonComparator(COMPARATOR_SHOW_ORIGINAL_ORDER);
         model.setDisplayClient(editedPerson);
         return new CommandResult(String.format(MESSAGE_EDIT_PERSON_SUCCESS, Messages.format(editedPerson)));
     }

src/main/java/seedu/address/logic/commands/FindCommand.java

@@ -30,13 +30,9 @@ public FindCommand(NameContainsKeywordsPredicate predicate) {
     public CommandResult execute(Model model) {
         requireNonNull(model);
         model.updateFilteredPersonList(predicate);
-        if (model.getFilteredPersonList().isEmpty()) {
-            model.setDisplayClient(null);
-        } else {
-            model.setDisplayClient(model.getFilteredPersonList().get(0));
-        }
+        model.setDisplayClientAsFirstInSortedFilteredPersonList();
         return new CommandResult(
-                String.format(Messages.MESSAGE_PERSONS_LISTED_OVERVIEW, model.getFilteredPersonList().size()));
+                String.format(Messages.MESSAGE_PERSONS_LISTED_OVERVIEW, model.getSortedFilteredPersonList().size()));
     }
 
     @Override

src/main/java/seedu/address/logic/commands/LastMetCommand.java

@@ -2,6 +2,7 @@
 
 import static java.util.Objects.requireNonNull;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_LASTMET;
+import static seedu.address.model.Model.COMPARATOR_SHOW_ORIGINAL_ORDER;
 import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 
 import java.time.LocalDate;
@@ -56,7 +57,7 @@ public CommandResult execute(Model model) throws CommandException {
             throw new CommandException(Messages.MESSAGE_LASTMET_FUTURE);
         }
 
-        List<Person> lastShownList = model.getFilteredPersonList();
+        List<Person> lastShownList = model.getSortedFilteredPersonList();
 
         if (index.getZeroBased() >= lastShownList.size()) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
@@ -71,6 +72,7 @@ public CommandResult execute(Model model) throws CommandException {
 
         model.setPerson(personToMeet, metPerson);
         model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        model.updateSortPersonComparator(COMPARATOR_SHOW_ORIGINAL_ORDER);
         model.setDisplayClient(metPerson);
 
         return new CommandResult(String.format(MESSAGE_SUCCESS, personToMeet.getName()));

src/main/java/seedu/address/logic/commands/ListCommand.java

@@ -1,6 +1,7 @@
 package seedu.address.logic.commands;
 
 import static java.util.Objects.requireNonNull;
+import static seedu.address.model.Model.COMPARATOR_SHOW_ORIGINAL_ORDER;
 import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 
 import seedu.address.model.Model;
@@ -19,11 +20,8 @@ public class ListCommand extends Command {
     public CommandResult execute(Model model) {
         requireNonNull(model);
         model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
-        if (model.getFilteredPersonList().isEmpty()) {
-            model.setDisplayClient(null);
-        } else {
-            model.setDisplayClient(model.getFilteredPersonList().get(0));
-        }
+        model.updateSortPersonComparator(COMPARATOR_SHOW_ORIGINAL_ORDER);
+        model.setDisplayClientAsFirstInSortedFilteredPersonList();
         return new CommandResult(MESSAGE_SUCCESS);
     }
 }

src/main/java/seedu/address/logic/commands/MarkCommand.java

@@ -1,6 +1,7 @@
 package seedu.address.logic.commands;
 
 import static java.util.Objects.requireNonNull;
+import static seedu.address.model.Model.COMPARATOR_SHOW_ORIGINAL_ORDER;
 import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 
 import java.util.List;
@@ -40,7 +41,7 @@ public MarkCommand(Index index) {
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
 
-        List<Person> lastShownList = model.getFilteredPersonList();
+        List<Person> lastShownList = model.getSortedFilteredPersonList();
 
         if (index.getZeroBased() >= lastShownList.size()) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
@@ -63,6 +64,7 @@ public CommandResult execute(Model model) throws CommandException {
         metPerson.getSchedule().markIsDone();
         model.setPerson(personToMeet, metPerson);
         model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        model.updateSortPersonComparator(COMPARATOR_SHOW_ORIGINAL_ORDER);
         model.setDisplayClient(metPerson);
 
         return new CommandResult(String.format(MESSAGE_SUCCESS, personToMeet.getName()));

src/main/java/seedu/address/logic/commands/RemarkCommand.java

@@ -2,6 +2,7 @@
 
 import static seedu.address.commons.util.CollectionUtil.requireAllNonNull;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_REMARK;
+import static seedu.address.model.Model.COMPARATOR_SHOW_ORIGINAL_ORDER;
 import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 
 import java.util.List;
@@ -48,7 +49,7 @@ public RemarkCommand(Index index, Remark remark) {
     }
     @Override
     public CommandResult execute(Model model) throws CommandException {
-        List<Person> lastShownList = model.getFilteredPersonList();
+        List<Person> lastShownList = model.getSortedFilteredPersonList();
 
         if (index.getZeroBased() >= lastShownList.size()) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
@@ -62,6 +63,7 @@ public CommandResult execute(Model model) throws CommandException {
 
         model.setPerson(personToEdit, editedPerson);
         model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        model.updateSortPersonComparator(COMPARATOR_SHOW_ORIGINAL_ORDER);
         model.setDisplayClient(editedPerson);
 
         return new CommandResult(generateSuccessMessage(editedPerson));

src/main/java/seedu/address/logic/commands/ScheduleCommand.java

@@ -2,6 +2,7 @@
 
 import static java.util.Objects.requireNonNull;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_SCHEDULE;
+import static seedu.address.model.Model.COMPARATOR_SHOW_ORIGINAL_ORDER;
 import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 
 import java.time.LocalDateTime;
@@ -56,7 +57,7 @@ public CommandResult execute(Model model) throws CommandException {
             throw new CommandException(Messages.MESSAGE_SCHEDULE_PAST);
         }
 
-        List<Person> lastShownList = model.getFilteredPersonList();
+        List<Person> lastShownList = model.getSortedFilteredPersonList();
 
         if (index.getZeroBased() >= lastShownList.size()) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
@@ -71,6 +72,7 @@ public CommandResult execute(Model model) throws CommandException {
 
         model.setPerson(personToMeet, metPerson);
         model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        model.updateSortPersonComparator(COMPARATOR_SHOW_ORIGINAL_ORDER);
         model.setDisplayClient(metPerson);
 
         return new CommandResult(String.format(MESSAGE_SUCCESS, personToMeet.getName()));

src/main/java/seedu/address/logic/commands/SortCommand.java

@@ -44,7 +44,8 @@ public SortCommand(SortCriteria sortCriteria, SortOrder sortOrder) {
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        model.sortFilteredPersonList(PersonComparator.getComparator(sortCriteria, sortOrder));
+        model.updateSortPersonComparator(PersonComparator.getComparator(sortCriteria, sortOrder));
+        model.setDisplayClientAsFirstInSortedFilteredPersonList();
         return new CommandResult(getMessageSuccess(sortCriteria, sortOrder));
     }
 

src/main/java/seedu/address/logic/commands/ViewCommand.java

@@ -36,7 +36,7 @@ public ViewCommand(Index targetIndex) {
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
-        List<Person> lastShownList = model.getFilteredPersonList();
+        List<Person> lastShownList = model.getSortedFilteredPersonList();
 
         if (targetIndex.getZeroBased() >= lastShownList.size()) {
             throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);

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

@@ -16,6 +16,8 @@ public interface Model {
     /** {@code Predicate} that always evaluate to true */
     Predicate<Person> PREDICATE_SHOW_ALL_PERSONS = unused -> true;
 
+    Comparator<Person> COMPARATOR_SHOW_ORIGINAL_ORDER = (person1, person2) -> 0;
+
     /**
      * Replaces user prefs data with the data in {@code userPrefs}.
      */
@@ -79,7 +81,7 @@ public interface Model {
     void setPerson(Person target, Person editedPerson);
 
     /** Returns an unmodifiable view of the filtered person list */
-    ObservableList<Person> getFilteredPersonList();
+    ObservableList<Person> getSortedFilteredPersonList();
 
     /**
      * Updates the filter of the filtered person list to filter by the given {@code predicate}.
@@ -91,7 +93,7 @@ public interface Model {
      * Sorts the filtered person list by the given {@code comparator}.
      * @throws NullPointerException if {@code comparator} is null.
      */
-    void sortFilteredPersonList(Comparator<Person> comparator);
+    void updateSortPersonComparator(Comparator<Person> comparator);
 
     /**
      * Returns the client to be displayed in ClientViewPanel.
@@ -112,6 +114,9 @@ public interface Model {
      * Replaces the current client to be displayed to {@code person}.
      */
     void setDisplayClient(Person person);
+
+    void setDisplayClientAsFirstInSortedFilteredPersonList();
+
     /**
      * Returns the reminder list for the overdue last met to be displayed in RemindersPanel.
      */

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

@@ -10,6 +10,7 @@
 
 import javafx.collections.ObservableList;
 import javafx.collections.transformation.FilteredList;
+import javafx.collections.transformation.SortedList;
 import seedu.address.commons.core.GuiSettings;
 import seedu.address.commons.core.LogsCenter;
 import seedu.address.model.person.DisplayClient;
@@ -28,7 +29,8 @@ public class ModelManager implements Model {
 
     private final AddressBook addressBook;
     private final UserPrefs userPrefs;
-    private FilteredList<Person> filteredPersons;
+    private final FilteredList<Person> filteredPersons;
+    private Comparator<Person> personComparator;
     private final DisplayClient displayClient;
 
     /**
@@ -42,6 +44,7 @@ public ModelManager(ReadOnlyAddressBook addressBook, ReadOnlyUserPrefs userPrefs
         this.addressBook = new AddressBook(addressBook);
         this.userPrefs = new UserPrefs(userPrefs);
         filteredPersons = new FilteredList<>(this.addressBook.getPersonList());
+        personComparator = COMPARATOR_SHOW_ORIGINAL_ORDER;
         displayClient = filteredPersons.isEmpty()
                 ? new DisplayClient(null)
                 : new DisplayClient(filteredPersons.get(0));
@@ -113,6 +116,7 @@ public void deletePerson(Person target) {
     public void addPerson(Person person) {
         addressBook.addPerson(person);
         updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        updateSortPersonComparator(COMPARATOR_SHOW_ORIGINAL_ORDER);
     }
 
     @Override
@@ -157,8 +161,10 @@ public void deletePolicy(Person target, String policyId) {
      * {@code versionedAddressBook}
      */
     @Override
-    public ObservableList<Person> getFilteredPersonList() {
-        return filteredPersons;
+    public ObservableList<Person> getSortedFilteredPersonList() {
+        SortedList<Person> sortedPersons = new SortedList<>(filteredPersons);
+        sortedPersons.setComparator(personComparator);
+        return sortedPersons;
     }
 
     @Override
@@ -167,11 +173,16 @@ public void updateFilteredPersonList(Predicate<Person> predicate) {
         filteredPersons.setPredicate(predicate);
     }
 
+    /**
+     * Updates the comparator used to sort the list of persons.
+     * Comparator is used to sort when {@code getSortedFilteredPersonList()} is called.
+     *
+     * @param comparator Comparator to be used to sort the list of persons
+     */
     @Override
-    public void sortFilteredPersonList(Comparator<Person> comparator) {
+    public void updateSortPersonComparator(Comparator<Person> comparator) {
         requireNonNull(comparator);
-        ObservableList<Person> sortedList = this.addressBook.getPersonList().sorted(comparator);
-        filteredPersons = new FilteredList<>(sortedList);
+        personComparator = comparator;
     }
 
     @Override
@@ -212,6 +223,20 @@ public void setDisplayClient(Person person) {
         displayClient.setDisplayClient(person);
     }
 
+    /**
+     * Sets the display client to be the first person in the sorted filtered person list.
+     * If the list is empty, display client is set to null.
+     */
+    @Override
+    public void setDisplayClientAsFirstInSortedFilteredPersonList() {
+        ObservableList<Person> sortedFilteredPersonList = getSortedFilteredPersonList();
+        if (sortedFilteredPersonList.isEmpty()) {
+            displayClient.setDisplayClient(null);
+        } else {
+            displayClient.setDisplayClient(sortedFilteredPersonList.get(0));
+        }
+    }
+
     //=========== PolicyList Displayed =====================================================================
     @Override
     public ReminderList getOverDueLastMet() {