Merge pull request #109 from solomonng2001/fix-birthday-sorting

Fix birthday sorting regardless of year bug

Author: ReganChoy

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

@@ -149,6 +149,6 @@ public ObservableList<Person> getSchedules() {
      * @return list of persons who have upcoming birthdays
      */
     public ObservableList<Person> getPersonsWithUpcomingBirthdays() {
-        return persons.filterHaveUpcomingBirthday();
+        return persons.sortFilterHaveUpcomingBirthday();
     }
 }

src/main/java/seedu/address/model/person/Birthday.java

@@ -80,4 +80,16 @@ public int hashCode() {
     public int compareTo(Birthday other) {
         return date.compareTo(other.date);
     }
+
+    /**
+     * Compares the month and date of this birthday with another birthday.
+     * Ignores the year.
+     *
+     * @param other The other birthday to compare with
+     * @return A negative integer, zero, or a positive integer as this birthday is before, at the same time, or after
+     */
+    public int compareByBirthdayMonthAndDateOnly(Birthday other) {
+        int currentYear = LocalDate.now().getYear();
+        return date.withYear(currentYear).compareTo(other.date.withYear(currentYear));
+    }
 }

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

@@ -9,6 +9,8 @@
 
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
+import javafx.collections.transformation.FilteredList;
+import javafx.collections.transformation.SortedList;
 import seedu.address.model.person.exceptions.DuplicatePersonException;
 import seedu.address.model.person.exceptions.PersonNotFoundException;
 
@@ -184,14 +186,13 @@ public ObservableList<Person> getSchedules() {
      * Returns the list of persons who have upcoming birthdays, sorted by birthday.
      * @return list of persons who have upcoming birthdays
      */
-    public ObservableList<Person> filterHaveUpcomingBirthday() {
-        ObservableList<Person> personsWithUpcomingBirthdays = FXCollections.observableArrayList();
-        for (Person person : internalList) {
-            if (person.hasUpcomingBirthday()) {
-                personsWithUpcomingBirthdays.add(person);
-            }
-        }
-        personsWithUpcomingBirthdays.sort(Comparator.comparing(Person::getBirthday));
-        return personsWithUpcomingBirthdays;
+    public ObservableList<Person> sortFilterHaveUpcomingBirthday() {
+        FilteredList<Person> filteredPersonsWithUpcomingBirthdays = new FilteredList<>(internalList);
+        filteredPersonsWithUpcomingBirthdays.setPredicate(Person::hasUpcomingBirthday);
+        SortedList<Person> sortedFilteredPersonsWithUpcomingBirthdays =
+                new SortedList<>(filteredPersonsWithUpcomingBirthdays);
+        sortedFilteredPersonsWithUpcomingBirthdays.setComparator(
+                Comparator.comparing(Person::getBirthday, Birthday::compareByBirthdayMonthAndDateOnly));
+        return sortedFilteredPersonsWithUpcomingBirthdays;
     }
 }

src/test/java/seedu/address/model/person/BirthdayTest.java

@@ -1,5 +1,6 @@
 package seedu.address.model.person;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static seedu.address.testutil.Assert.assertThrows;
@@ -121,4 +122,24 @@ public void equals() {
         // different values -> returns false
         assertFalse(birthday.equals(new Birthday("1993-09-23")));
     }
+
+    @Test
+    public void compareByBirthdayMonthAndDateOnly() {
+        Birthday birthday = new Birthday("1993-09-22");
+
+        // same values -> returns 0 regardless of year
+        assertEquals(0, birthday.compareByBirthdayMonthAndDateOnly(new Birthday("1998-09-22")));
+
+        // different month -> returns negative integer regardless of year
+        assertTrue(birthday.compareByBirthdayMonthAndDateOnly(new Birthday("1992-10-22")) < 0);
+
+        // different day -> returns negative integer regardless of year
+        assertTrue(birthday.compareByBirthdayMonthAndDateOnly(new Birthday("1991-09-23")) < 0);
+
+        // different month -> returns positive integer regardless of year
+        assertTrue(birthday.compareByBirthdayMonthAndDateOnly(new Birthday("2023-08-22")) > 0);
+
+        // different day -> returns positive integer regardless of year
+        assertTrue(birthday.compareByBirthdayMonthAndDateOnly(new Birthday("2008-09-01")) > 0);
+    }
 }

src/test/java/seedu/address/model/person/UniquePersonListTest.java

@@ -7,6 +7,7 @@
 import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_HUSBAND;
 import static seedu.address.testutil.Assert.assertThrows;
 import static seedu.address.testutil.TypicalPersons.ALICE;
+import static seedu.address.testutil.TypicalPersons.BENSON;
 import static seedu.address.testutil.TypicalPersons.BOB;
 
 import java.time.LocalDate;
@@ -178,6 +179,29 @@ public void toStringMethod() {
 
     @Test
     public void filterHaveUpcomingBirthday() {
+        Person personWithUpcomingBirthdaySecond = new PersonBuilder(ALICE).withBirthday(
+                DateUtil.parseDateToString(LocalDate.now().plusWeeks(1)
+                        .minusYears(40))).build();
+        Person personWithNoUpcomingBirthday = new PersonBuilder(BOB).withBirthday(
+                DateUtil.parseDateToString(LocalDate.now().minusDays(1)
+                        .minusYears(30))).build();
+        Person personWithUpcomingBirthdayFirst = new PersonBuilder(BENSON).withBirthday(
+                DateUtil.parseDateToString(LocalDate.now().plusDays(3)
+                        .minusYears(30))).build();
+
+        UniquePersonList newPersonsList = new UniquePersonList();
+        newPersonsList.add(personWithUpcomingBirthdaySecond);
+        newPersonsList.add(personWithNoUpcomingBirthday);
+        newPersonsList.add(personWithUpcomingBirthdayFirst);
+        ObservableList<Person> birthdayList = newPersonsList.sortFilterHaveUpcomingBirthday();
+
+        assertEquals(2, birthdayList.size());
+        assertEquals(personWithUpcomingBirthdayFirst, birthdayList.get(0));
+        assertEquals(personWithUpcomingBirthdaySecond, birthdayList.get(1));
+    }
+
+    @Test
+    public void sortFilterHaveUpcomingBirthday() {
         Person personWithUpcomingBirthday = new PersonBuilder(ALICE).withBirthday(
                 DateUtil.parseDateToString(LocalDate.now().plusWeeks(1)
                         .minusYears(40))).build();
@@ -188,7 +212,7 @@ public void filterHaveUpcomingBirthday() {
         UniquePersonList newPersonsList = new UniquePersonList();
         newPersonsList.add(personWithUpcomingBirthday);
         newPersonsList.add(personWithNoUpcomingBirthday);
-        ObservableList<Person> birthdayList = newPersonsList.filterHaveUpcomingBirthday();
+        ObservableList<Person> birthdayList = newPersonsList.sortFilterHaveUpcomingBirthday();
 
         assertEquals(1, birthdayList.size());
         assertTrue(birthdayList.contains(personWithUpcomingBirthday));