From e4101c1bd5c1e31ed3fc3e7f3cd426e8fd8a525a Mon Sep 17 00:00:00 2001 From: jylow Date: Wed, 8 Nov 2023 00:13:41 +0800 Subject: [PATCH] Fix sort command bug and User Guide documentation Currently, The sort command has a bug where the ordering of persons with null appointments will change each time the function is called. This bug fix fixes the functionality and causes people will null appointments to be ordered in an arbitrary order based on the previous way the list was sorted. Edits were also made to the user guide to provide further clarification to the ordering of peoples names when sort appointment is called. --- docs/UserGuide.md | 5 ++--- docs/team/jylow.md | 12 +++++++++--- .../address/model/appointment/Appointment.java | 2 +- .../address/model/appointment/NullAppointment.java | 6 +++++- .../address/logic/commands/SortCommandTest.java | 14 +++++++------- 5 files changed, 24 insertions(+), 15 deletions(-) diff --git a/docs/UserGuide.md b/docs/UserGuide.md index 5a137f6ffb7..fe884886cfa 100644 --- a/docs/UserGuide.md +++ b/docs/UserGuide.md @@ -373,13 +373,12 @@ Example: ### Sorting of data : `sort` Sorts all the entries with predefined sorting functionalities. After sorting the list, the ordering of the entries -will be changed. As a result, performing delete operations that require indexing will reference the new ordering -that is currently displayed on the screen. +will be changed. As a result, performing any operations that require indexing (such as delete, add or edit), will reference the new ordering that is currently displayed on the screen. **Here are the current predefined sorting functions that have been implemented** * `name` : sorts list by lexicographical ordering of name (case-insensitive). -* `appointment`: sorts list by appointment timing in order of the earliest appointment first. +* `appointment`: sorts list by appointment timing in order of the earliest appointment first. If no appointment is found, the remaining persons without an appointment will be displayed in an arbitrary order, based on the reordering of the previous sorting functions applied. Format: `sort KEYWORD` diff --git a/docs/team/jylow.md b/docs/team/jylow.md index 48d086a8b25..ed1cf22fbc2 100644 --- a/docs/team/jylow.md +++ b/docs/team/jylow.md @@ -15,10 +15,16 @@ Given below are my contributions to the project. * What it does: allows the user to perform sorting of list by appointment time and lexicographical order of name. * Justification: This feature improves the product significantly because a user can more efficiently find clients by name and the proximity of their appointments to view upcoming appointments. * Highlights: This enhancement creates a base to implement different sorting capabilities in the future through sorting by new comparators. It required an understanding of ObservableList interface and the way the list is being tracked by JavaFX. - + +* **New Feature**: Added confirm override window for `schedule` command if person already has a current appointment + * What it does: Caution a user if he intends to schedule a new appointment when there is already one that is not yet complete. + * Justification: This feature reduces the likelihood of overwriting an appointment if a person already has one. Most people only arrange one appointment at a time. So the design prevents multiple appointments and also serves as a reminder of a previously set appointment that might have been forgotten. + * Highlights: This feature causes the logic flow of the method to change if there is currently an appointment and results in breaking the execution into 2, getting a response from the user before deciding whether to continue the execution of the program. * **Code Contributed**: [RepoSense](https://nus-cs2103-ay2324s1.github.io/tp-dashboard/?search=jylow&breakdown=true) -* **Enhancements Implemented**: Sort Command to enable the list to be sorted by lexicographical order of name of client +* **Enhancements Implemented**: + * Sort Command to enable the list to be sorted by lexicographical order of name of client + * Override prompt to ask user for confirmation before overriding an appointment. * **Contributions to the UG**: * Added documentation for the features `sort` [\#81](https://github.com/AY2324S1-CS2103T-F12-1/tp/pull/81) @@ -28,4 +34,4 @@ Given below are my contributions to the project. * **Community**: * PRs reviewed (with non-trivial review comments): [\#125](https://github.com/AY2324S1-CS2103T-F12-1/tp/pull/125) [\#133](https://github.com/AY2324S1-CS2103T-F12-1/tp/pull/133#pullrequestreview-1699166607) -* **Contributions to team-based tasks**: to be added soon. +* **Contributions to team-based tasks**: Released version 1.3 of the application diff --git a/src/main/java/seedu/address/model/appointment/Appointment.java b/src/main/java/seedu/address/model/appointment/Appointment.java index 1e62a441137..ad8111d3866 100644 --- a/src/main/java/seedu/address/model/appointment/Appointment.java +++ b/src/main/java/seedu/address/model/appointment/Appointment.java @@ -140,7 +140,7 @@ public void setPerson(Person person) { @Override public int compareTo(ScheduleItem scheduleItem) { if (scheduleItem instanceof NullAppointment) { - return 0; + return -1; //person with appointment should be smaller to be sorted up on the list } else { Appointment appointment = (Appointment) scheduleItem; return this.date.compareTo(appointment.date); diff --git a/src/main/java/seedu/address/model/appointment/NullAppointment.java b/src/main/java/seedu/address/model/appointment/NullAppointment.java index 12cb87546b6..950e9e6138a 100644 --- a/src/main/java/seedu/address/model/appointment/NullAppointment.java +++ b/src/main/java/seedu/address/model/appointment/NullAppointment.java @@ -32,7 +32,11 @@ public int hashCode() { @Override public int compareTo(ScheduleItem appointment) { - return 1; + if (appointment == this) { + return 0; + } else { + return 1; //null appointment returns >0 so it will be sorted further down the list + } } @Override diff --git a/src/test/java/seedu/address/logic/commands/SortCommandTest.java b/src/test/java/seedu/address/logic/commands/SortCommandTest.java index 0bcde1c9ea4..d0949703a7b 100644 --- a/src/test/java/seedu/address/logic/commands/SortCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/SortCommandTest.java @@ -45,22 +45,22 @@ public void equals() { SortByNameComparator nameComparator = new SortByNameComparator(); SortByAppointmentComparator appointmentComparator = new SortByAppointmentComparator(); - SortCommand sortSortCommand = new SortCommand(nameComparator); + SortCommand sortNameCommand = new SortCommand(nameComparator); SortCommand appointmentSortCommand = new SortCommand(appointmentComparator); // same object -> returns true - assertTrue(sortSortCommand.equals(sortSortCommand)); + assertTrue(sortNameCommand.equals(sortNameCommand)); assertTrue(appointmentSortCommand.equals(appointmentSortCommand)); // different types -> returns false - assertFalse(sortSortCommand.equals(1)); + assertFalse(sortNameCommand.equals(1)); // null -> returns false - assertFalse(sortSortCommand.equals(null)); + assertFalse(sortNameCommand.equals(null)); // different command -> returns false - assertFalse(sortSortCommand.equals(appointmentSortCommand)); - assertFalse(appointmentSortCommand.equals(sortSortCommand)); + assertFalse(sortNameCommand.equals(appointmentSortCommand)); + assertFalse(appointmentSortCommand.equals(sortNameCommand)); } @Test @@ -79,7 +79,7 @@ public void execute_sortAppointmentCommand() { expectedModel.sortFilteredPersonList(comparator); assertCommandSuccess(command, model, expectedMessage, expectedModel); assertEquals(model.getAppointmentList(), expectedModel.getAppointmentList()); - assertEquals(Arrays.asList(CARL, BENSON, GEORGE, FIONA, ELLE, DANIEL, ALICE), model.getFilteredPersonList()); + assertEquals(Arrays.asList(CARL, BENSON, ALICE, DANIEL, ELLE, FIONA, GEORGE), model.getFilteredPersonList()); } }