From 9d4ee3f5bccfa47f96de7d037139658ae1e0af91 Mon Sep 17 00:00:00 2001 From: wz2k <87819279+wz2k@users.noreply.github.com> Date: Sat, 8 Apr 2023 20:00:08 +0800 Subject: [PATCH 1/2] Refactor edit commands --- .../address/logic/commands/EditCommand.java | 18 +++-- .../logic/commands/EditElderlyCommand.java | 71 +++++++++++++------ .../logic/commands/EditVolunteerCommand.java | 68 ++++++++++++------ .../logic/commands/EditCommandTest.java | 3 +- 4 files changed, 105 insertions(+), 55 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/EditCommand.java b/src/main/java/seedu/address/logic/commands/EditCommand.java index 8b37feb4586..cdb4f978f5c 100644 --- a/src/main/java/seedu/address/logic/commands/EditCommand.java +++ b/src/main/java/seedu/address/logic/commands/EditCommand.java @@ -104,9 +104,9 @@ public CommandResult execute(Model model) throws CommandException { private CommandResult editElderly(Model model) throws CommandException { Elderly elderlyToEdit = model.getElderly(nric); - Elderly editedElderly = EditDescriptor.createEditedElderly( - elderlyToEdit, editDescriptor); + Elderly editedElderly = EditDescriptor.createEditedElderly(elderlyToEdit, editDescriptor); Nric editedNric = editedElderly.getNric(); + if (!elderlyToEdit.isSamePerson(editedElderly) && model.hasElderly(editedNric)) { throw new CommandException(MESSAGE_DUPLICATE_PERSON_IN_ELDERLY); } @@ -114,10 +114,9 @@ private CommandResult editElderly(Model model) throws CommandException { throw new CommandException(MESSAGE_DUPLICATE_PERSON_IN_VOLUNTEERS); } - model.setElderly(elderlyToEdit, editedElderly); - model.refreshAllFilteredLists(); - return new CommandResult(String.format( - EditElderlyCommand.MESSAGE_EDIT_ELDERLY_SUCCESS, editedElderly)); + String finalMessage = EditElderlyCommand.updateElderly( + model, elderlyToEdit, editedElderly); + return new CommandResult(finalMessage); } private CommandResult editVolunteer(Model model) throws CommandException { @@ -132,10 +131,9 @@ private CommandResult editVolunteer(Model model) throws CommandException { throw new CommandException(MESSAGE_DUPLICATE_PERSON_IN_ELDERLY); } - model.setVolunteer(volunteerToEdit, editedVolunteer); - model.refreshAllFilteredLists(); - return new CommandResult(String.format( - EditVolunteerCommand.MESSAGE_EDIT_VOLUNTEER_SUCCESS, editedVolunteer)); + String finalMessage = EditVolunteerCommand.updateVolunteer( + model, volunteerToEdit, editedVolunteer); + return new CommandResult(finalMessage); } @Override diff --git a/src/main/java/seedu/address/logic/commands/EditElderlyCommand.java b/src/main/java/seedu/address/logic/commands/EditElderlyCommand.java index e8b085f3ea9..21d98431155 100644 --- a/src/main/java/seedu/address/logic/commands/EditElderlyCommand.java +++ b/src/main/java/seedu/address/logic/commands/EditElderlyCommand.java @@ -92,24 +92,45 @@ public EditElderlyCommand(Index index, EditDescriptor editDescriptor) { this.editDescriptor = new EditDescriptor(editDescriptor); } - @Override - public CommandResult execute(Model model) throws CommandException { - if (!editDescriptor.isAnyFieldEdited()) { - throw new CommandException(MESSAGE_NO_FIELD_PROVIDED); + /** + * Replaces the elderly that has been edited with the edited elderly. + * + * @param model FriendlyLink's model. + * @param elderlyToEdit Elderly to replace. + * @param editedElderly Elderly with new fields. + * @return Command result message. + */ + public static String updateElderly(Model model, Elderly elderlyToEdit, Elderly editedElderly) { + model.setElderly(elderlyToEdit, editedElderly); + + @SuppressWarnings("unchecked") + Predicate predicate = (Predicate) PREDICATE_SHOW_ALL; + model.updateFilteredElderlyList(predicate); + String finalMessage = String.format(MESSAGE_EDIT_ELDERLY_SUCCESS, editedElderly); + + if (!model.check(editedElderly, Person::isSuitableRegion)) { + finalMessage += MESSAGE_WARNING_REGION; } + if (!model.check(editedElderly, Person::hasSuitableAvailableDates)) { + finalMessage += MESSAGE_WARNING_AVAILABLE_DATES; + } + + model.refreshAllFilteredLists(); + return finalMessage; + } + @Override + public CommandResult execute(Model model) throws CommandException { requireNonNull(model); - List lastShownList = model.getFilteredElderlyList(); - if (index.getZeroBased() >= lastShownList.size()) { - throw new CommandException(MESSAGE_INVALID_ELDERLY_DISPLAYED_INDEX); + if (!editDescriptor.isAnyFieldEdited()) { + throw new CommandException(MESSAGE_NO_FIELD_PROVIDED); } - assert index.getZeroBased() >= 0 : "index should not be negative"; - Elderly elderlyToEdit = lastShownList.get(index.getZeroBased()); + Elderly elderlyToEdit = getElderlyToEdit(model); Elderly editedElderly = EditDescriptor.createEditedElderly(elderlyToEdit, editDescriptor); - Nric editedNric = editedElderly.getNric(); + if (!elderlyToEdit.isSamePerson(editedElderly) && model.hasElderly(editedNric)) { throw new CommandException(MESSAGE_DUPLICATE_PERSON_IN_ELDERLY); } @@ -117,24 +138,28 @@ public CommandResult execute(Model model) throws CommandException { throw new CommandException(MESSAGE_DUPLICATE_PERSON_IN_VOLUNTEERS); } - model.setElderly(elderlyToEdit, editedElderly); - @SuppressWarnings("unchecked") - Predicate predicate = (Predicate) PREDICATE_SHOW_ALL; - model.updateFilteredElderlyList(predicate); + String finalMessage = updateElderly(model, elderlyToEdit, editedElderly); + return new CommandResult(finalMessage); + } - String finalMessage = String.format(MESSAGE_EDIT_ELDERLY_SUCCESS, editedElderly); - if (!model.check(editedElderly, Person::isSuitableRegion)) { - finalMessage += MESSAGE_WARNING_REGION; - } - if (!model.check(editedElderly, Person::hasSuitableAvailableDates)) { - finalMessage += MESSAGE_WARNING_AVAILABLE_DATES; + /** + * Returns the elderly that has been edited. + * + * @param model FriendlyLink's model. + * @return Elderly that has been edited. + * @throws CommandException If index is invalid. + */ + private Elderly getElderlyToEdit(Model model) throws CommandException { + List lastShownList = model.getFilteredElderlyList(); + + if (index.getZeroBased() >= lastShownList.size()) { + throw new CommandException(MESSAGE_INVALID_ELDERLY_DISPLAYED_INDEX); } + assert index.getZeroBased() >= 0 : "index should not be negative"; - model.refreshAllFilteredLists(); - return new CommandResult(finalMessage); + return lastShownList.get(index.getZeroBased()); } - @Override public boolean equals(Object other) { // short circuit if same object diff --git a/src/main/java/seedu/address/logic/commands/EditVolunteerCommand.java b/src/main/java/seedu/address/logic/commands/EditVolunteerCommand.java index 7b74a140af1..b08839a4782 100644 --- a/src/main/java/seedu/address/logic/commands/EditVolunteerCommand.java +++ b/src/main/java/seedu/address/logic/commands/EditVolunteerCommand.java @@ -92,21 +92,42 @@ public EditVolunteerCommand(Index index, EditDescriptor editDescriptor) { this.editDescriptor = new EditDescriptor(editDescriptor); } - @Override - public CommandResult execute(Model model) throws CommandException { - if (!editDescriptor.isAnyFieldEdited()) { - throw new CommandException(MESSAGE_NO_FIELD_PROVIDED); + /** + * Replaces the volunteer that has been edited with the edited volunteer. + * + * @param model FriendlyLink's model. + * @param volunteerToEdit Volunteer to replace. + * @param editedVolunteer Volunteer with new fields. + * @return Command result message. + */ + public static String updateVolunteer(Model model, Volunteer volunteerToEdit, Volunteer editedVolunteer) { + model.setVolunteer(volunteerToEdit, editedVolunteer); + + @SuppressWarnings("unchecked") + Predicate predicate = (Predicate) PREDICATE_SHOW_ALL; + model.updateFilteredVolunteerList(predicate); + String finalMessage = String.format(MESSAGE_EDIT_VOLUNTEER_SUCCESS, editedVolunteer); + + if (!model.check(editedVolunteer, Person::isSuitableRegion)) { + finalMessage += MESSAGE_WARNING_REGION; + } + if (!model.check(editedVolunteer, Person::hasSuitableAvailableDates)) { + finalMessage += MESSAGE_WARNING_AVAILABLE_DATES; } + model.refreshAllFilteredLists(); + return finalMessage; + } + + @Override + public CommandResult execute(Model model) throws CommandException { requireNonNull(model); - List lastShownList = model.getFilteredVolunteerList(); - if (index.getZeroBased() >= lastShownList.size()) { - throw new CommandException(MESSAGE_INVALID_VOLUNTEER_DISPLAYED_INDEX); + if (!editDescriptor.isAnyFieldEdited()) { + throw new CommandException(MESSAGE_NO_FIELD_PROVIDED); } - assert index.getZeroBased() >= 0 : "index should not be negative"; - Volunteer volunteerToEdit = lastShownList.get(index.getZeroBased()); + Volunteer volunteerToEdit = getVolunteerToEdit(model); Volunteer editedVolunteer = EditDescriptor.createEditedVolunteer( volunteerToEdit, editDescriptor); @@ -118,21 +139,26 @@ public CommandResult execute(Model model) throws CommandException { throw new CommandException(MESSAGE_DUPLICATE_PERSON_IN_ELDERLY); } - model.setVolunteer(volunteerToEdit, editedVolunteer); - @SuppressWarnings("unchecked") - Predicate predicate = (Predicate) PREDICATE_SHOW_ALL; - model.updateFilteredVolunteerList(predicate); + String finalMessage = updateVolunteer(model, volunteerToEdit, editedVolunteer); + return new CommandResult(finalMessage); + } - String finalMessage = String.format(MESSAGE_EDIT_VOLUNTEER_SUCCESS, editedVolunteer); - if (!model.check(editedVolunteer, Person::isSuitableRegion)) { - finalMessage += MESSAGE_WARNING_REGION; - } - if (!model.check(editedVolunteer, Person::hasSuitableAvailableDates)) { - finalMessage += MESSAGE_WARNING_AVAILABLE_DATES; + /** + * Returns the volunteer that has been edited. + * + * @param model FriendlyLink's model. + * @return Volunteer that has been edited. + * @throws CommandException If index is invalid. + */ + private Volunteer getVolunteerToEdit(Model model) throws CommandException { + List lastShownList = model.getFilteredVolunteerList(); + + if (index.getZeroBased() >= lastShownList.size()) { + throw new CommandException(MESSAGE_INVALID_VOLUNTEER_DISPLAYED_INDEX); } + assert index.getZeroBased() >= 0 : "index should not be negative"; - model.refreshAllFilteredLists(); - return new CommandResult(finalMessage); + return lastShownList.get(index.getZeroBased()); } @Override diff --git a/src/test/java/seedu/address/logic/commands/EditCommandTest.java b/src/test/java/seedu/address/logic/commands/EditCommandTest.java index 1fe5fd1ee63..ff3d13295fe 100644 --- a/src/test/java/seedu/address/logic/commands/EditCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/EditCommandTest.java @@ -6,6 +6,7 @@ import static seedu.address.commons.core.Messages.MESSAGE_DUPLICATE_PERSON_IN_VOLUNTEERS; import static seedu.address.commons.core.Messages.MESSAGE_NO_FIELD_PROVIDED; import static seedu.address.commons.core.Messages.MESSAGE_NRIC_NOT_EXIST; +import static seedu.address.commons.core.Messages.MESSAGE_WARNING_REGION; import static seedu.address.logic.commands.CommandTestUtil.DESC_AMY; import static seedu.address.logic.commands.CommandTestUtil.DESC_BOB; import static seedu.address.logic.commands.CommandTestUtil.VALID_NRIC_AMY; @@ -42,7 +43,7 @@ public void execute_validElderlyNric_success() { EditCommand editCommand = new EditCommand(selectedNric, descriptor); String expectedMessage = String.format(EditElderlyCommand.MESSAGE_EDIT_ELDERLY_SUCCESS, - resultantElderly); + resultantElderly) + MESSAGE_WARNING_REGION; Model expectedModel = new ModelManagerBuilder() .withFriendlyLink(model.getFriendlyLink()) From 6633679ce52b36e0673ff3fa4ed967292deed610 Mon Sep 17 00:00:00 2001 From: wz2k <87819279+wz2k@users.noreply.github.com> Date: Sat, 8 Apr 2023 20:00:24 +0800 Subject: [PATCH 2/2] Remove dead code --- .../address/model/person/information/RiskLevelTest.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/test/java/seedu/address/model/person/information/RiskLevelTest.java b/src/test/java/seedu/address/model/person/information/RiskLevelTest.java index 2b737e2c635..1a8d51ae382 100644 --- a/src/test/java/seedu/address/model/person/information/RiskLevelTest.java +++ b/src/test/java/seedu/address/model/person/information/RiskLevelTest.java @@ -9,11 +9,6 @@ class RiskLevelTest { - // @Test // RiskLevel does not allow RiskLevel(null) constructor - // public void constructor_null_throwsNullPointerException() { - // assertThrows(NullPointerException.class, () -> new RiskLevel(null)); - // } - @Test public void constructor_invalidRisk_throwsIllegalArgumentException() { String invalidRisk = "abc";