From 130ff3a955f60d34dbffae6867c1a50356d43442 Mon Sep 17 00:00:00 2001 From: zhonghan721 Date: Sat, 11 Nov 2023 20:14:07 +0800 Subject: [PATCH] Improve code quality --- src/main/java/seedu/address/logic/Messages.java | 2 +- .../commands/customer/CustomerAddCommand.java | 14 ++++++++++++-- .../commands/customer/CustomerFindCommand.java | 9 ++++++++- .../logic/commands/user/UserLoginCommand.java | 10 ++++++++-- .../logic/commands/user/UserLogoutCommand.java | 15 +++++++++------ .../logic/commands/user/UserUpdateCommand.java | 5 +++++ .../customer/CustomerAddCommandParser.java | 10 +++++++--- .../parser/user/UserLoginCommandParser.java | 5 +++++ .../parser/user/UserUpdateCommandParser.java | 7 +++++++ src/main/java/seedu/address/model/Model.java | 10 ++++++++++ .../java/seedu/address/model/ModelManager.java | 16 ++++++++++++++++ .../seedu/address/model/customer/Customer.java | 9 ++++++++- src/main/java/seedu/address/model/user/User.java | 11 ----------- .../logic/commands/DeliveryAddCommandTest.java | 16 ++++++++++++++++ .../customer/CustomerAddCommandTest.java | 16 ++++++++++++++++ .../java/seedu/address/model/user/UserTest.java | 7 +++++++ 16 files changed, 135 insertions(+), 27 deletions(-) diff --git a/src/main/java/seedu/address/logic/Messages.java b/src/main/java/seedu/address/logic/Messages.java index 57186f53280..0c45d59f284 100644 --- a/src/main/java/seedu/address/logic/Messages.java +++ b/src/main/java/seedu/address/logic/Messages.java @@ -22,7 +22,7 @@ public class Messages { public static final String MESSAGE_CUSTOMERS_MATCHED_LISTED = "%1$d customers listed!"; public static final String MESSAGE_DELIVERY_LISTED_OVERVIEW = "%1$d deliveries listed!"; public static final String MESSAGE_DUPLICATE_FIELDS = "Multiple values specified for the following" - + "single-valued field(s): "; + + " single-valued field(s): "; public static final String MESSAGE_USER_NOT_AUTHENTICATED = "Access denied! You are currently not logged in."; public static final String MESSAGE_WELCOME = "Welcome to HomeBoss!\n\n" + "You are currently logged out.\n" + "Login or register to start using HomeBoss."; diff --git a/src/main/java/seedu/address/logic/commands/customer/CustomerAddCommand.java b/src/main/java/seedu/address/logic/commands/customer/CustomerAddCommand.java index 65fe9a874aa..228d89cdef0 100644 --- a/src/main/java/seedu/address/logic/commands/customer/CustomerAddCommand.java +++ b/src/main/java/seedu/address/logic/commands/customer/CustomerAddCommand.java @@ -7,6 +7,8 @@ import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME; import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE; +import java.util.logging.Logger; + import seedu.address.commons.util.ToStringBuilder; import seedu.address.logic.Messages; import seedu.address.logic.commands.CommandResult; @@ -35,6 +37,7 @@ public class CustomerAddCommand extends CustomerCommand { public static final String MESSAGE_SUCCESS = "New customer added:\n\n%1$s"; public static final String MESSAGE_DUPLICATE_CUSTOMER = "This customer already exists in HomeBoss"; + private static final Logger logger = Logger.getLogger(CustomerAddCommand.class.getName()); private final Customer toAdd; @@ -49,17 +52,24 @@ public CustomerAddCommand(Customer customer) { @Override public CommandResult execute(Model model) throws CommandException { requireNonNull(model); + logger.info("Executing CustomerAddCommand: name " + + toAdd.getName() + ", phone " + + toAdd.getPhone() + ", email " + + toAdd.getEmail() + ", address " + + toAdd.getAddress()); // User cannot perform this operation before logging in if (!model.getUserLoginStatus()) { // reset the customer count to the previous value - Customer.setCustomerCount(toAdd.getCustomerId() - 1); + Customer.resetPrevCustomerCount(); + logger.warning("User is not logged in!"); throw new CommandException(MESSAGE_USER_NOT_AUTHENTICATED); } if (model.hasCustomer(toAdd)) { // reset the customer count to the previous value - Customer.setCustomerCount(toAdd.getCustomerId() - 1); + Customer.resetPrevCustomerCount(); + logger.warning("Duplicate customer found!"); throw new CommandException(MESSAGE_DUPLICATE_CUSTOMER); } diff --git a/src/main/java/seedu/address/logic/commands/customer/CustomerFindCommand.java b/src/main/java/seedu/address/logic/commands/customer/CustomerFindCommand.java index 50730ba7278..7a59ad3f16b 100644 --- a/src/main/java/seedu/address/logic/commands/customer/CustomerFindCommand.java +++ b/src/main/java/seedu/address/logic/commands/customer/CustomerFindCommand.java @@ -3,6 +3,8 @@ import static java.util.Objects.requireNonNull; import static seedu.address.logic.Messages.MESSAGE_USER_NOT_AUTHENTICATED; +import java.util.logging.Logger; + import seedu.address.commons.util.ToStringBuilder; import seedu.address.logic.Messages; import seedu.address.logic.commands.Command; @@ -24,6 +26,8 @@ public class CustomerFindCommand extends Command { + "Parameters: KEYWORD [MORE_KEYWORDS]...\n\n" + "Example: " + COMMAND_WORD + " alice bob charlie"; + private static final Logger logger = Logger.getLogger(CustomerFindCommand.class.getName()); + private final NameContainsKeywordsPredicate predicate; public CustomerFindCommand(NameContainsKeywordsPredicate predicate) { @@ -33,16 +37,19 @@ public CustomerFindCommand(NameContainsKeywordsPredicate predicate) { @Override public CommandResult execute(Model model) throws CommandException { requireNonNull(model); + logger.info("Executing CustomerFindCommand: keyword " + + predicate.getKeywordsAsString()); // User cannot perform this operation before logging in if (!model.getUserLoginStatus()) { + logger.warning("User is not logged in!"); throw new CommandException(MESSAGE_USER_NOT_AUTHENTICATED); } model.updateFilteredCustomerList(predicate); return new CommandResult( String.format(Messages.MESSAGE_CUSTOMERS_MATCHED_LISTED, - model.getFilteredCustomerList().size()), true); + model.getFilteredCustomerListSize()), true); } diff --git a/src/main/java/seedu/address/logic/commands/user/UserLoginCommand.java b/src/main/java/seedu/address/logic/commands/user/UserLoginCommand.java index 642f813edde..2851789e593 100644 --- a/src/main/java/seedu/address/logic/commands/user/UserLoginCommand.java +++ b/src/main/java/seedu/address/logic/commands/user/UserLoginCommand.java @@ -3,7 +3,8 @@ import static java.util.Objects.requireNonNull; import static seedu.address.logic.parser.CliSyntax.PREFIX_PASSWORD; import static seedu.address.logic.parser.CliSyntax.PREFIX_USER; -import static seedu.address.model.Model.PREDICATE_SHOW_ALL_CUSTOMERS; + +import java.util.logging.Logger; import seedu.address.logic.commands.Command; import seedu.address.logic.commands.CommandResult; @@ -31,6 +32,7 @@ public class UserLoginCommand extends Command { public static final String MESSAGE_NO_REGISTERED_ACCOUNT_FOUND = "No registered account found.\n\n" + "Please register an account first.\n\n" + UserRegisterCommand.MESSAGE_USAGE; + private static final Logger logger = Logger.getLogger(UserLoginCommand.class.getName()); private final User user; @@ -52,24 +54,28 @@ public UserLoginCommand(User user) { @Override public CommandResult execute(Model model) throws CommandException { requireNonNull(model); + logger.info("Executing UserLoginCommand: user " + user.getUsername()); // Check if there is a stored user if (model.getStoredUser() == null) { + logger.warning("No registered account found!"); throw new CommandException(MESSAGE_NO_REGISTERED_ACCOUNT_FOUND); } // Logged in user cannot login again if (model.getUserLoginStatus()) { + logger.warning("User is already logged in!"); throw new CommandException(MESSAGE_ALREADY_LOGGED_IN); } // Check if the user matches the user loaded in model if (!model.userMatches(user)) { + logger.warning("User credentials does not match!"); throw new CommandException(MESSAGE_WRONG_CREDENTIALS); } model.setLoginSuccess(); - model.updateFilteredCustomerList(PREDICATE_SHOW_ALL_CUSTOMERS); + model.showAllFilteredCustomerList(); return new CommandResult(MESSAGE_SUCCESS, true); } diff --git a/src/main/java/seedu/address/logic/commands/user/UserLogoutCommand.java b/src/main/java/seedu/address/logic/commands/user/UserLogoutCommand.java index b6c6d17d661..f91f33b42db 100644 --- a/src/main/java/seedu/address/logic/commands/user/UserLogoutCommand.java +++ b/src/main/java/seedu/address/logic/commands/user/UserLogoutCommand.java @@ -1,8 +1,8 @@ package seedu.address.logic.commands.user; import static java.util.Objects.requireNonNull; -import static seedu.address.model.Model.PREDICATE_SHOW_NO_CUSTOMERS; -import static seedu.address.model.Model.PREDICATE_SHOW_NO_DELIVERIES; + +import java.util.logging.Logger; import seedu.address.logic.commands.Command; import seedu.address.logic.commands.CommandResult; @@ -17,22 +17,25 @@ public class UserLogoutCommand extends Command { public static final String COMMAND_WORD = "logout"; public static final String MESSAGE_SUCCESS = "Bye!"; public static final String MESSAGE_ALREADY_LOGGED_OUT = "You are already logged out!"; + private static final Logger logger = Logger.getLogger(UserLogoutCommand.class.getName()); + @Override public CommandResult execute(Model model) throws CommandException { requireNonNull(model); + logger.info("Executing UserLogoutCommand"); // Logged out user cannot log out again if (!model.getUserLoginStatus()) { + logger.warning("User is already logged out!"); throw new CommandException(MESSAGE_ALREADY_LOGGED_OUT); } // Set status in model to logged out, and update filtered lists model.setLogoutSuccess(); - model.updateFilteredCustomerList(PREDICATE_SHOW_NO_CUSTOMERS); - model.updateFilteredDeliveryList(PREDICATE_SHOW_NO_DELIVERIES); - // Display the updated empty list - model.setUiListCustomer(); + model.clearFilteredDeliveryList(); + model.clearFilteredCustomerList(); + return new CommandResult(MESSAGE_SUCCESS, true); } } diff --git a/src/main/java/seedu/address/logic/commands/user/UserUpdateCommand.java b/src/main/java/seedu/address/logic/commands/user/UserUpdateCommand.java index c78d8268433..7da1c23b645 100644 --- a/src/main/java/seedu/address/logic/commands/user/UserUpdateCommand.java +++ b/src/main/java/seedu/address/logic/commands/user/UserUpdateCommand.java @@ -10,6 +10,7 @@ import java.util.Objects; import java.util.Optional; +import java.util.logging.Logger; import seedu.address.commons.util.CollectionUtil; import seedu.address.commons.util.ToStringBuilder; @@ -45,6 +46,8 @@ public class UserUpdateCommand extends Command { public static final String MESSAGE_PASSWORD_MISMATCH = "Passwords do not match. Please try again."; public static final String MESSAGE_QUESTION_OR_ANSWER_MISSING = "Secret Question and Answer have to be " + "either all present or all absent. Try again."; + private static final Logger logger = Logger.getLogger(UserUpdateCommand.class.getName()); + private final UserUpdateDescriptor userUpdateDescriptor; /** @@ -65,9 +68,11 @@ public UserUpdateCommand(UserUpdateDescriptor userUpdateDescriptor) { @Override public CommandResult execute(Model model) throws CommandException { requireNonNull(model); + logger.info("Executing UserUpdateCommand"); // User cannot perform this operation before logging in if (!model.getUserLoginStatus()) { + logger.warning("User is not logged in!"); throw new CommandException(MESSAGE_USER_NOT_AUTHENTICATED); } diff --git a/src/main/java/seedu/address/logic/parser/customer/CustomerAddCommandParser.java b/src/main/java/seedu/address/logic/parser/customer/CustomerAddCommandParser.java index 7e1d8164224..6f52cb783f0 100644 --- a/src/main/java/seedu/address/logic/parser/customer/CustomerAddCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/customer/CustomerAddCommandParser.java @@ -6,6 +6,7 @@ import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME; import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE; +import java.util.logging.Logger; import java.util.stream.Stream; import seedu.address.logic.commands.customer.CustomerAddCommand; @@ -22,13 +23,15 @@ import seedu.address.model.customer.Phone; /** - * Parses input arguments and creates a new AddCommand object + * Parses input arguments and creates a new CustomerAddCommand object */ public class CustomerAddCommandParser implements Parser { + private static final Logger logger = Logger.getLogger(CustomerAddCommandParser.class.getName()); + /** - * Parses the given {@code String} of arguments in the context of the AddCommand - * and returns an AddCommand object for execution. + * Parses the given {@code String} of arguments in the context of the CustomerAddCommand + * and returns a CustomerAddCommand object for execution. * * @throws ParseException if the user input does not conform the expected format */ @@ -38,6 +41,7 @@ public CustomerAddCommand parse(String args) throws ParseException { if (!arePrefixesPresent(argMultimap, PREFIX_NAME, PREFIX_ADDRESS, PREFIX_PHONE, PREFIX_EMAIL) || !argMultimap.getPreamble().isEmpty()) { + logger.severe("Could not parse command"); throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, CustomerAddCommand.MESSAGE_USAGE)); } diff --git a/src/main/java/seedu/address/logic/parser/user/UserLoginCommandParser.java b/src/main/java/seedu/address/logic/parser/user/UserLoginCommandParser.java index 76cbb66375d..f3bd2bcdbc8 100644 --- a/src/main/java/seedu/address/logic/parser/user/UserLoginCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/user/UserLoginCommandParser.java @@ -4,6 +4,7 @@ import static seedu.address.logic.parser.CliSyntax.PREFIX_PASSWORD; import static seedu.address.logic.parser.CliSyntax.PREFIX_USER; +import java.util.logging.Logger; import java.util.stream.Stream; import seedu.address.logic.commands.user.UserLoginCommand; @@ -21,6 +22,9 @@ * Parses input arguments and creates a new UserLoginCommand object */ public class UserLoginCommandParser implements Parser { + + private static final Logger logger = Logger.getLogger(UserLoginCommandParser.class.getName()); + /** * Parses the given {@code String} of arguments in the context of the UserLoginCommand * and returns an UserLoginCommand object for execution. @@ -33,6 +37,7 @@ public UserLoginCommand parse(String args) throws ParseException { if (!arePrefixesPresent(argMultimap, PREFIX_USER, PREFIX_PASSWORD) || !argMultimap.getPreamble().isEmpty()) { + logger.severe("Could not parse command"); throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, UserLoginCommand.MESSAGE_USAGE)); } diff --git a/src/main/java/seedu/address/logic/parser/user/UserUpdateCommandParser.java b/src/main/java/seedu/address/logic/parser/user/UserUpdateCommandParser.java index 6aa52281c9f..71b4760b685 100644 --- a/src/main/java/seedu/address/logic/parser/user/UserUpdateCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/user/UserUpdateCommandParser.java @@ -7,6 +7,8 @@ import static seedu.address.logic.parser.CliSyntax.PREFIX_SECRET_QUESTION; import static seedu.address.logic.parser.CliSyntax.PREFIX_USER; +import java.util.logging.Logger; + import seedu.address.logic.commands.user.UserUpdateCommand; import seedu.address.logic.commands.user.UserUpdateCommand.UserUpdateDescriptor; import seedu.address.logic.parser.ArgumentMultimap; @@ -20,6 +22,9 @@ * Parses input arguments and creates a new UserUpdateCommand object */ public class UserUpdateCommandParser implements Parser { + + private static final Logger logger = Logger.getLogger(UserUpdateCommandParser.class.getName()); + /** * Parses the given {@code String} of arguments in the context of the UserUpdateCommand * and returns a UserUpdateCommand object for execution. @@ -37,6 +42,7 @@ public UserUpdateCommand parse(String args) throws ParseException { UserUpdateDescriptor userUpdateDescriptor = new UserUpdateDescriptor(); if (!argMultimap.getPreamble().isEmpty()) { + logger.severe("Could not parse command"); throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, UserUpdateCommand.MESSAGE_USAGE)); } @@ -49,6 +55,7 @@ public UserUpdateCommand parse(String args) throws ParseException { userUpdateDescriptor = parseSecretQuestionAndAnswer(argMultimap, userUpdateDescriptor); if (!userUpdateDescriptor.isAnyFieldEdited()) { + logger.warning("No fields provided"); throw new ParseException( String.format(UserUpdateCommand.MESSAGE_MISSING_FIELDS, UserUpdateCommand.MESSAGE_USAGE)); } diff --git a/src/main/java/seedu/address/model/Model.java b/src/main/java/seedu/address/model/Model.java index 310c2c41299..1bf74089823 100644 --- a/src/main/java/seedu/address/model/Model.java +++ b/src/main/java/seedu/address/model/Model.java @@ -145,6 +145,11 @@ public interface Model { */ void showAllFilteredCustomerList(); + /** + * Resets the customer list to show no customers. + */ + void clearFilteredCustomerList(); + /** * Returns the number of customers in the filtered customer list. * @@ -225,6 +230,11 @@ public interface Model { */ void showAllFilteredDeliveryList(); + /** + * Resets the delivery list to show no deliveries. + */ + void clearFilteredDeliveryList(); + /** * Returns the number of deliveries in the filtered delivery list. * diff --git a/src/main/java/seedu/address/model/ModelManager.java b/src/main/java/seedu/address/model/ModelManager.java index 9ede358a14d..1daec905eb4 100644 --- a/src/main/java/seedu/address/model/ModelManager.java +++ b/src/main/java/seedu/address/model/ModelManager.java @@ -280,6 +280,14 @@ public void showAllFilteredCustomerList() { updateFilteredCustomerList(PREDICATE_SHOW_ALL_CUSTOMERS); } + /** + * Resets the customer list to show no customers. + */ + @Override + public void clearFilteredCustomerList() { + updateFilteredCustomerList(PREDICATE_SHOW_NO_CUSTOMERS); + } + /** * Returns the number of customers in the filtered customer list. * @@ -463,6 +471,14 @@ public void showAllFilteredDeliveryList() { updateFilteredDeliveryList(PREDICATE_SHOW_ALL_DELIVERIES); } + /** + * Resets the delivery list to show no deliveries. + */ + @Override + public void clearFilteredDeliveryList() { + updateFilteredDeliveryList(PREDICATE_SHOW_NO_DELIVERIES); + } + /** * Returns the number of deliveries in the filtered delivery list. * diff --git a/src/main/java/seedu/address/model/customer/Customer.java b/src/main/java/seedu/address/model/customer/Customer.java index 1bd0dbb8a37..d762df68da4 100644 --- a/src/main/java/seedu/address/model/customer/Customer.java +++ b/src/main/java/seedu/address/model/customer/Customer.java @@ -81,7 +81,7 @@ public static void setCustomerCount(int count) { /** * Returns current customerCount. - * Used by {@code CustomerBuilder} to create customer with expected customerId. + * To be used by {@code CustomerBuilder} to create customer with expected customerId. * * @return customerCount */ @@ -89,6 +89,13 @@ public static int getCustomerCount() { return Customer.customerCount; } + /** + * Decrements customerCount by 1. + */ + public static void resetPrevCustomerCount() { + Customer.customerCount -= 1; + } + /** * Returns true if both Customers have the same customerId or {@code Phone}. * This defines a weaker notion of equality between two customers. diff --git a/src/main/java/seedu/address/model/user/User.java b/src/main/java/seedu/address/model/user/User.java index b67abbb70bc..c18e095f081 100644 --- a/src/main/java/seedu/address/model/user/User.java +++ b/src/main/java/seedu/address/model/user/User.java @@ -18,17 +18,6 @@ public class User { private String secretQuestion; private String answer; - - /** - * Every field must be present and not null. - * This constructor assumes a non-hashed password is passed in - */ - public User(Username username, Password password) { - requireAllNonNull(username, password); - this.username = username; - this.hashedPassword = new Password(password.toString()); - } - /** * Overloaded constructor for creating a new user with a hashed password * diff --git a/src/test/java/seedu/address/logic/commands/DeliveryAddCommandTest.java b/src/test/java/seedu/address/logic/commands/DeliveryAddCommandTest.java index d36c5ba7b19..65cb8fc0afa 100644 --- a/src/test/java/seedu/address/logic/commands/DeliveryAddCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/DeliveryAddCommandTest.java @@ -274,6 +274,14 @@ public void showAllFilteredCustomerList() { throw new AssertionError("This method should not be called."); } + /** + * Resets the customer list to show no customers. + */ + @Override + public void clearFilteredCustomerList() { + throw new AssertionError("This method should not be called."); + } + /** * Returns the number of customers in the filtered customer list. * @@ -352,6 +360,14 @@ public void showAllFilteredDeliveryList() { throw new AssertionError("This method should not be called."); } + /** + * Resets the delivery list to show no deliveries. + */ + @Override + public void clearFilteredDeliveryList() { + throw new AssertionError("This method should not be called."); + } + /** * Returns the number of deliveries in the filtered delivery list. * diff --git a/src/test/java/seedu/address/logic/commands/customer/CustomerAddCommandTest.java b/src/test/java/seedu/address/logic/commands/customer/CustomerAddCommandTest.java index 0241d661a8d..d70dc7ae725 100644 --- a/src/test/java/seedu/address/logic/commands/customer/CustomerAddCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/customer/CustomerAddCommandTest.java @@ -216,6 +216,14 @@ public void showAllFilteredCustomerList() { throw new AssertionError("This method should not be called."); } + /** + * Resets the customer list to show no customers. + */ + @Override + public void clearFilteredCustomerList() { + throw new AssertionError("This method should not be called."); + } + /** * Returns the number of customers in the filtered customer list. * @@ -294,6 +302,14 @@ public void showAllFilteredDeliveryList() { throw new AssertionError("This method should not be called."); } + /** + * Resets the delivery list to show no deliveries. + */ + @Override + public void clearFilteredDeliveryList() { + throw new AssertionError("This method should not be called."); + } + /** * Returns the number of deliveries in the filtered delivery list. * diff --git a/src/test/java/seedu/address/model/user/UserTest.java b/src/test/java/seedu/address/model/user/UserTest.java index 8fdbcd8bb6d..a8758f6137d 100644 --- a/src/test/java/seedu/address/model/user/UserTest.java +++ b/src/test/java/seedu/address/model/user/UserTest.java @@ -1,5 +1,6 @@ package seedu.address.model.user; +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.TypicalUsers.AARON; @@ -58,4 +59,10 @@ public void equals() { User newAaron = new UserBuilder(AARON).build(); assertTrue(AARON.equals(newAaron)); } + + @Test + public void toStringMethod() { + String expected = User.class.getCanonicalName() + "{username=" + AARON.getUsername() + "}"; + assertEquals(expected, AARON.toString()); + } }