Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[T2A2][W13-A3]Melvin Tan #304

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+61 −51
Open

[T2A2][W13-A3]Melvin Tan #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
60b1263
Add DS_Store to gitignore
Melvin-Tan Feb 3, 2017
d1247bb
Private class variables can have underscore prefix
Melvin-Tan Feb 4, 2017
eb4a464
The explicit //Fallthrough comment should be included whenever there …
Melvin-Tan Feb 4, 2017
5cc8758
Refactor method to check if filename has an extension
Melvin-Tan Feb 4, 2017
2755b95
Refactor method removePrefixSign to removePrefix
Melvin-Tan Feb 4, 2017
297dd8d
Minor changes
Melvin-Tan Feb 4, 2017
94dea61
Change LS to LINE_SEPARATOR
Melvin-Tan Feb 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,6 @@ test/actual.txt
test/localrun.bat
test/data/
/bin/

# DS_Store
.DS_Store
109 changes: 58 additions & 51 deletions src/seedu/addressbook/AddressBook.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 : ==================================================================
Expand All @@ -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";
Expand Down Expand Up @@ -188,12 +188,12 @@ public class AddressBook {
* This is a subset of the full list. Deleting persons in the pull list does not delete
* those persons from this list.
*/
private static ArrayList<String[]> latestPersonListingView = getAllPersonsInAddressBook(); // initial view is of all
private static ArrayList<String[]> _latestPersonListingView = getAllPersonsInAddressBook(); // initial view is of all

/**
* The path to the file used for storing person data.
*/
private static String storageFilePath;
private static String _storageFilePath;

/*
* NOTE : =============================================================
Expand Down Expand Up @@ -283,7 +283,7 @@ private static void setupGivenFileForStorage(String filePath) {
exitProgram();
}

storageFilePath = filePath;
_storageFilePath = filePath;
createFileIfMissing(filePath);
}

Expand All @@ -302,8 +302,8 @@ private static void exitProgram() {
*/
private static void setupDefaultFileForStorage() {
showToUser(MESSAGE_USING_DEFAULT_FILE);
storageFilePath = DEFAULT_STORAGE_FILEPATH;
createFileIfMissing(storageFilePath);
_storageFilePath = DEFAULT_STORAGE_FILEPATH;
createFileIfMissing(_storageFilePath);
}

/**
Expand Down Expand Up @@ -339,16 +339,23 @@ private static boolean hasValidParentDirectory(Path filePath) {
* If a file already exists, it must be a regular file.
*/
private static boolean hasValidFileName(Path filePath) {
return filePath.getFileName().toString().lastIndexOf('.') > 0
return hasExtensionForFilename(filePath)
&& (!Files.exists(filePath) || Files.isRegularFile(filePath));
}

/**
* Returns true if file path have a file name that has an extension.
*/
private static boolean hasExtensionForFilename(Path filePath) {
return filePath.getFileName().toString().lastIndexOf('.') > 0;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about refactoring the magic string and numbers here

}

/**
* Initialises the in-memory data using the storage file.
* Assumption: The file exists.
*/
private static void loadDataFromStorage() {
initialiseAddressBookModel(loadPersonsFromFile(storageFilePath));
initialiseAddressBookModel(loadPersonsFromFile(_storageFilePath));
}


Expand Down Expand Up @@ -383,6 +390,7 @@ private static String executeCommand(String userInputString) {
return getUsageInfoForAllCommands();
case COMMAND_EXIT_WORD:
executeExitProgramRequest();
// Fallthrough
default:
return getMessageForInvalidCommandInput(commandType, getUsageInfoForAllCommands());
}
Expand Down Expand Up @@ -544,7 +552,7 @@ private static int extractTargetIndexFromDeletePersonArgs(String rawArgs) {
* @return whether it is valid
*/
private static boolean isDisplayIndexValidForLastPersonListingView(int index) {
return index >= DISPLAYED_INDEX_OFFSET && index < latestPersonListingView.size() + DISPLAYED_INDEX_OFFSET;
return index >= DISPLAYED_INDEX_OFFSET && index < _latestPersonListingView.size() + DISPLAYED_INDEX_OFFSET;
}

/**
Expand Down Expand Up @@ -645,7 +653,7 @@ private static String getDisplayString(ArrayList<String[]> persons) {
final int displayIndex = i + DISPLAYED_INDEX_OFFSET;
messageAccumulator.append('\t')
.append(getIndexedPersonListElementMessage(displayIndex, person))
.append(LS);
.append(LINE_SEPARATOR);
}
return messageAccumulator.toString();
}
Expand Down Expand Up @@ -679,7 +687,7 @@ private static String getMessageForFormattedPersonData(String[] person) {
*/
private static void updateLatestViewedPersonListing(ArrayList<String[]> newListing) {
// clone to insulate from future changes to arg list
latestPersonListingView = new ArrayList<>(newListing);
_latestPersonListingView = new ArrayList<>(newListing);
}

/**
Expand All @@ -689,7 +697,7 @@ private static void updateLatestViewedPersonListing(ArrayList<String[]> newListi
* @return the actual person object in the last shown person listing
*/
private static String[] getPersonByLastVisibleIndex(int lastVisibleIndex) {
return latestPersonListingView.get(lastVisibleIndex - DISPLAYED_INDEX_OFFSET);
return _latestPersonListingView.get(lastVisibleIndex - DISPLAYED_INDEX_OFFSET);
}


Expand Down Expand Up @@ -763,7 +771,7 @@ private static ArrayList<String> getLinesInFile(String filePath) {
private static void savePersonsToFile(ArrayList<String[]> persons, String filePath) {
final ArrayList<String> linesToWrite = encodePersonsToStrings(persons);
try {
Files.write(Paths.get(storageFilePath), linesToWrite);
Files.write(Paths.get(_storageFilePath), linesToWrite);
} catch (IOException ioe) {
showToUser(String.format(MESSAGE_ERROR_WRITING_TO_FILE, filePath));
exitProgram();
Expand All @@ -784,7 +792,7 @@ private static void savePersonsToFile(ArrayList<String[]> persons, String filePa
*/
private static void addPersonToAddressBook(String[] person) {
ALL_PERSONS.add(person);
savePersonsToFile(getAllPersonsInAddressBook(), storageFilePath);
savePersonsToFile(getAllPersonsInAddressBook(), _storageFilePath);
}

/**
Expand All @@ -796,7 +804,7 @@ private static void addPersonToAddressBook(String[] person) {
private static boolean deletePersonFromAddressBook(String[] exactPerson) {
final boolean changed = ALL_PERSONS.remove(exactPerson);
if (changed) {
savePersonsToFile(getAllPersonsInAddressBook(), storageFilePath);
savePersonsToFile(getAllPersonsInAddressBook(), _storageFilePath);
}
return changed;
}
Expand All @@ -813,7 +821,7 @@ private static ArrayList<String[]> getAllPersonsInAddressBook() {
*/
private static void clearAddressBook() {
ALL_PERSONS.clear();
savePersonsToFile(getAllPersonsInAddressBook(), storageFilePath);
savePersonsToFile(getAllPersonsInAddressBook(), _storageFilePath);
}

/**
Expand Down Expand Up @@ -989,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);
}
Expand All @@ -1012,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);
}
Expand Down Expand Up @@ -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 */
Expand All @@ -1142,16 +1150,15 @@ private static String getUsageInfoForExitCommand() {
* UTILITY METHODS
* ============================
*/

/**
* Removes sign(p/, d/, etc) from parameter string
*
* @param s Parameter as a string
* @param sign Parameter sign to be removed
* @return string without the sign
*/
private static String removePrefixSign(String s, String sign) {
return s.replace(sign, "");
* Removes prefix from the given fullString if prefix occurs at the start of the string.
* @param fullString Parameter as a string
* @param prefix Parameter prefix to be removed
* @return string without the prefix
*/
private static String removePrefix(String fullString, String prefix) {
return fullString.startsWith(prefix) ? fullString.substring(prefix.length()) : fullString;
}

/**
Expand Down