From c29b1b8b22e65a37306677409b6d7fe027aa9e32 Mon Sep 17 00:00:00 2001 From: "MYPC\\Fifer" Date: Wed, 8 Feb 2017 01:40:35 +0800 Subject: [PATCH 1/2] Refactoring and improve code quality --- src/seedu/addressbook/AddressBook.java | 74 ++++++++++++++------------ 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/src/seedu/addressbook/AddressBook.java b/src/seedu/addressbook/AddressBook.java index 5a158b67..20da0a3f 100644 --- a/src/seedu/addressbook/AddressBook.java +++ b/src/seedu/addressbook/AddressBook.java @@ -54,7 +54,7 @@ public class AddressBook { /** * A platform independent line separator. */ - private static final String LS = System.lineSeparator() + LINE_PREFIX; + private static final String LINE_SEPARATOR = System.lineSeparator() + LINE_PREFIX; /* * NOTE : ================================================================== @@ -74,11 +74,11 @@ public class AddressBook { private static final String MESSAGE_DISPLAY_PERSON_DATA = "%1$s Phone Number: %2$s Email: %3$s"; private static final String MESSAGE_DISPLAY_LIST_ELEMENT_INDEX = "%1$d. "; private static final String MESSAGE_GOODBYE = "Exiting Address Book... Good bye!"; - private static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format: %1$s " + LS + "%2$s"; + private static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format: %1$s " + LINE_SEPARATOR + "%2$s"; private static final String MESSAGE_INVALID_FILE = "The given file name [%1$s] is not a valid file name!"; private static final String MESSAGE_INVALID_PROGRAM_ARGS = "Too many parameters! Correct program argument format:" - + LS + "\tjava AddressBook" - + LS + "\tjava AddressBook [custom storage file path]"; + + LINE_SEPARATOR + "\tjava AddressBook" + + LINE_SEPARATOR + "\tjava AddressBook [custom storage file path]"; private static final String MESSAGE_INVALID_PERSON_DISPLAYED_INDEX = "The person index provided is invalid"; private static final String MESSAGE_INVALID_STORAGE_FILE_CONTENT = "Storage file has invalid content"; private static final String MESSAGE_PERSON_NOT_IN_ADDRESSBOOK = "Person could not be found in address book"; @@ -154,6 +154,14 @@ public class AddressBook { * Offset required to convert between 1-indexing and 0-indexing.COMMAND_ */ private static final int DISPLAYED_INDEX_OFFSET = 1; + /** + * Command Type Index + */ + private static final int COMMAND_TYPE_INDEX = 0; + /** + * Command Argument Index + */ + private static final int COMMAND_ARGS_INDEX = 1; /** * If the first non-whitespace character in a user's input line is this, that line will be ignored. @@ -366,8 +374,8 @@ private static void loadDataFromStorage() { */ private static String executeCommand(String userInputString) { final String[] commandTypeAndParams = splitCommandWordAndArgs(userInputString); - final String commandType = commandTypeAndParams[0]; - final String commandArgs = commandTypeAndParams[1]; + final String commandType = commandTypeAndParams[COMMAND_TYPE_INDEX]; + final String commandArgs = commandTypeAndParams[COMMAND_ARGS_INDEX]; switch (commandType) { case COMMAND_ADD_WORD: return executeAddPerson(commandArgs); @@ -645,7 +653,7 @@ private static String getDisplayString(ArrayList persons) { final int displayIndex = i + DISPLAYED_INDEX_OFFSET; messageAccumulator.append('\t') .append(getIndexedPersonListElementMessage(displayIndex, person)) - .append(LS); + .append(LINE_SEPARATOR); } return messageAccumulator.toString(); } @@ -794,11 +802,11 @@ private static void addPersonToAddressBook(String[] person) { * @return true if the given person was found and deleted in the model */ private static boolean deletePersonFromAddressBook(String[] exactPerson) { - final boolean changed = ALL_PERSONS.remove(exactPerson); - if (changed) { + final boolean ischanged = ALL_PERSONS.remove(exactPerson); + if (ischanged) { savePersonsToFile(getAllPersonsInAddressBook(), storageFilePath); } - return changed; + return ischanged; } /** @@ -957,10 +965,10 @@ private static Optional> decodePersonsFromStrings(ArrayList< private static boolean isPersonDataExtractableFrom(String personData) { final String matchAnyPersonDataPrefix = PERSON_DATA_PREFIX_PHONE + '|' + PERSON_DATA_PREFIX_EMAIL; final String[] splitArgs = personData.trim().split(matchAnyPersonDataPrefix); - return splitArgs.length == 3 // 3 arguments - && !splitArgs[0].isEmpty() // non-empty arguments - && !splitArgs[1].isEmpty() - && !splitArgs[2].isEmpty(); + return splitArgs.length == PERSON_DATA_COUNT // 3 arguments + && !splitArgs[PERSON_DATA_INDEX_NAME].isEmpty() // non-empty arguments + && !splitArgs[PERSON_DATA_INDEX_PHONE].isEmpty() + && !splitArgs[PERSON_DATA_INDEX_EMAIL].isEmpty(); } /** @@ -1082,46 +1090,46 @@ private static boolean isPersonEmailValid(String email) { /** Returns usage info for all commands */ private static String getUsageInfoForAllCommands() { - return getUsageInfoForAddCommand() + LS - + getUsageInfoForFindCommand() + LS - + getUsageInfoForViewCommand() + LS - + getUsageInfoForDeleteCommand() + LS - + getUsageInfoForClearCommand() + LS - + getUsageInfoForExitCommand() + LS + return getUsageInfoForAddCommand() + LINE_SEPARATOR + + getUsageInfoForFindCommand() + LINE_SEPARATOR + + getUsageInfoForViewCommand() + LINE_SEPARATOR + + getUsageInfoForDeleteCommand() + LINE_SEPARATOR + + getUsageInfoForClearCommand() + LINE_SEPARATOR + + getUsageInfoForExitCommand() + LINE_SEPARATOR + getUsageInfoForHelpCommand(); } /** Returns the string for showing 'add' command usage instruction */ private static String getUsageInfoForAddCommand() { - return String.format(MESSAGE_COMMAND_HELP, COMMAND_ADD_WORD, COMMAND_ADD_DESC) + LS - + String.format(MESSAGE_COMMAND_HELP_PARAMETERS, COMMAND_ADD_PARAMETERS) + LS - + String.format(MESSAGE_COMMAND_HELP_EXAMPLE, COMMAND_ADD_EXAMPLE) + LS; + return String.format(MESSAGE_COMMAND_HELP, COMMAND_ADD_WORD, COMMAND_ADD_DESC) + LINE_SEPARATOR + + String.format(MESSAGE_COMMAND_HELP_PARAMETERS, COMMAND_ADD_PARAMETERS) + LINE_SEPARATOR + + String.format(MESSAGE_COMMAND_HELP_EXAMPLE, COMMAND_ADD_EXAMPLE) + LINE_SEPARATOR; } /** Returns the string for showing 'find' command usage instruction */ private static String getUsageInfoForFindCommand() { - return String.format(MESSAGE_COMMAND_HELP, COMMAND_FIND_WORD, COMMAND_FIND_DESC) + LS - + String.format(MESSAGE_COMMAND_HELP_PARAMETERS, COMMAND_FIND_PARAMETERS) + LS - + String.format(MESSAGE_COMMAND_HELP_EXAMPLE, COMMAND_FIND_EXAMPLE) + LS; + return String.format(MESSAGE_COMMAND_HELP, COMMAND_FIND_WORD, COMMAND_FIND_DESC) + LINE_SEPARATOR + + String.format(MESSAGE_COMMAND_HELP_PARAMETERS, COMMAND_FIND_PARAMETERS) + LINE_SEPARATOR + + String.format(MESSAGE_COMMAND_HELP_EXAMPLE, COMMAND_FIND_EXAMPLE) + LINE_SEPARATOR; } /** Returns the string for showing 'delete' command usage instruction */ private static String getUsageInfoForDeleteCommand() { - return String.format(MESSAGE_COMMAND_HELP, COMMAND_DELETE_WORD, COMMAND_DELETE_DESC) + LS - + String.format(MESSAGE_COMMAND_HELP_PARAMETERS, COMMAND_DELETE_PARAMETER) + LS - + String.format(MESSAGE_COMMAND_HELP_EXAMPLE, COMMAND_DELETE_EXAMPLE) + LS; + return String.format(MESSAGE_COMMAND_HELP, COMMAND_DELETE_WORD, COMMAND_DELETE_DESC) + LINE_SEPARATOR + + String.format(MESSAGE_COMMAND_HELP_PARAMETERS, COMMAND_DELETE_PARAMETER) + LINE_SEPARATOR + + String.format(MESSAGE_COMMAND_HELP_EXAMPLE, COMMAND_DELETE_EXAMPLE) + LINE_SEPARATOR; } /** Returns string for showing 'clear' command usage instruction */ private static String getUsageInfoForClearCommand() { - return String.format(MESSAGE_COMMAND_HELP, COMMAND_CLEAR_WORD, COMMAND_CLEAR_DESC) + LS - + String.format(MESSAGE_COMMAND_HELP_EXAMPLE, COMMAND_CLEAR_EXAMPLE) + LS; + return String.format(MESSAGE_COMMAND_HELP, COMMAND_CLEAR_WORD, COMMAND_CLEAR_DESC) + LINE_SEPARATOR + + String.format(MESSAGE_COMMAND_HELP_EXAMPLE, COMMAND_CLEAR_EXAMPLE) + LINE_SEPARATOR; } /** Returns the string for showing 'view' command usage instruction */ private static String getUsageInfoForViewCommand() { - return String.format(MESSAGE_COMMAND_HELP, COMMAND_LIST_WORD, COMMAND_LIST_DESC) + LS - + String.format(MESSAGE_COMMAND_HELP_EXAMPLE, COMMAND_LIST_EXAMPLE) + LS; + return String.format(MESSAGE_COMMAND_HELP, COMMAND_LIST_WORD, COMMAND_LIST_DESC) + LINE_SEPARATOR + + String.format(MESSAGE_COMMAND_HELP_EXAMPLE, COMMAND_LIST_EXAMPLE) + LINE_SEPARATOR; } /** Returns string for showing 'help' command usage instruction */ From 5f98df0b6d22761bb02fc2835e76bb52f7cc06f3 Mon Sep 17 00:00:00 2001 From: "MYPC\\Fifer" Date: Wed, 8 Feb 2017 02:22:48 +0800 Subject: [PATCH 2/2] Ready for Review --- src/seedu/addressbook/AddressBook.java | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/seedu/addressbook/AddressBook.java b/src/seedu/addressbook/AddressBook.java index 20da0a3f..dbab929c 100644 --- a/src/seedu/addressbook/AddressBook.java +++ b/src/seedu/addressbook/AddressBook.java @@ -220,7 +220,6 @@ public static void main(String[] args) { loadDataFromStorage(); while (true) { String userCommand = getUserInput(); - echoUserCommand(userCommand); String feedback = executeCommand(userCommand); showResultToUser(feedback); } @@ -373,6 +372,7 @@ private static void loadDataFromStorage() { * @return feedback about how the command was executed */ private static String executeCommand(String userInputString) { + echoUserCommand(userInputString); final String[] commandTypeAndParams = splitCommandWordAndArgs(userInputString); final String commandType = commandTypeAndParams[COMMAND_TYPE_INDEX]; final String commandArgs = commandTypeAndParams[COMMAND_ARGS_INDEX]; @@ -997,12 +997,12 @@ private static String extractPhoneFromPersonString(String encoded) { // phone is last arg, target is from prefix to end of string if (indexOfPhonePrefix > indexOfEmailPrefix) { - return removePrefixSign(encoded.substring(indexOfPhonePrefix, encoded.length()).trim(), + return removePrefix(encoded.substring(indexOfPhonePrefix, encoded.length()).trim(), PERSON_DATA_PREFIX_PHONE); // phone is middle arg, target is from own prefix to next prefix } else { - return removePrefixSign( + return removePrefix( encoded.substring(indexOfPhonePrefix, indexOfEmailPrefix).trim(), PERSON_DATA_PREFIX_PHONE); } @@ -1020,12 +1020,12 @@ private static String extractEmailFromPersonString(String encoded) { // email is last arg, target is from prefix to end of string if (indexOfEmailPrefix > indexOfPhonePrefix) { - return removePrefixSign(encoded.substring(indexOfEmailPrefix, encoded.length()).trim(), + return removePrefix(encoded.substring(indexOfEmailPrefix, encoded.length()).trim(), PERSON_DATA_PREFIX_EMAIL); // email is middle arg, target is from own prefix to next prefix } else { - return removePrefixSign( + return removePrefix( encoded.substring(indexOfEmailPrefix, indexOfPhonePrefix).trim(), PERSON_DATA_PREFIX_EMAIL); } @@ -1152,14 +1152,14 @@ private static String getUsageInfoForExitCommand() { */ /** - * Removes sign(p/, d/, etc) from parameter string + * Removes prefix(p/, d/, etc) from parameter string * - * @param s Parameter as a string - * @param sign Parameter sign to be removed - * @return string without the sign + * @param fullString Parameter as a string + * @param prefix Parameter prefix to be removed + * @return string without the prefix */ - private static String removePrefixSign(String s, String sign) { - return s.replace(sign, ""); + private static String removePrefix(String fullString, String prefix) { + return fullString.replace(prefix, ""); } /**