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.

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][W15-A1] Xu Bili #267

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
74 changes: 64 additions & 10 deletions src/seedu/addressbook/AddressBook.java
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ public static void main(String[] args) {
showWelcomeMessage();
processProgramArgs(args);
loadDataFromStorage();
startEventLoop();
}

private static void startEventLoop() {
while (true) {
String userCommand = getUserInput();
echoUserCommand(userCommand);
Expand Down Expand Up @@ -266,7 +270,7 @@ private static void processProgramArgs(String[] args) {
setupGivenFileForStorage(args[0]);
}

if(args.length == 0) {
if (args.length == 0) {
Copy link

Choose a reason for hiding this comment

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

good that coding standard violation was spotted.

setupDefaultFileForStorage();
}
}
Expand All @@ -277,7 +281,6 @@ private static void processProgramArgs(String[] args) {
* Exits if the file name is not acceptable.
*/
private static void setupGivenFileForStorage(String filePath) {

if (!isValidFilePath(filePath)) {
showToUser(String.format(MESSAGE_INVALID_FILE, filePath));
exitProgram();
Expand Down Expand Up @@ -315,12 +318,14 @@ private static boolean isValidFilePath(String filePath) {
if (filePath == null) {
return false;
}

Path filePathToValidate;
try {
filePathToValidate = Paths.get(filePath);
} catch (InvalidPathException ipe) {
return false;
}

return hasValidParentDirectory(filePathToValidate) && hasValidFileName(filePathToValidate);
}

Expand All @@ -329,6 +334,7 @@ private static boolean isValidFilePath(String filePath) {
*/
private static boolean hasValidParentDirectory(Path filePath) {
Path parentDirectory = filePath.getParent();

return parentDirectory == null || Files.isDirectory(parentDirectory);
}

Expand All @@ -339,8 +345,28 @@ 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
&& (!Files.exists(filePath) || Files.isRegularFile(filePath));
if (!hasExtension(filePath)) {
Copy link

Choose a reason for hiding this comment

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

good that the long condition has been made easier to read.

return false;
}

if (Files.exists(filePath)) {
return Files.isRegularFile(filePath);
}

return true;
}

/**
* Returns true if filePath has an extension
*/
private static boolean hasExtension(Path filePath) {
// checks if file name is valid with an extension
int extensionIndex = filePath.getFileName().toString().lastIndexOf('.');
if (extensionIndex <= 0) {
return false;
}

return true;
}

/**
Expand All @@ -366,8 +392,10 @@ 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];

switch (commandType) {
case COMMAND_ADD_WORD:
return executeAddPerson(commandArgs);
Expand Down Expand Up @@ -395,6 +423,7 @@ private static String executeCommand(String userInputString) {
*/
private static String[] splitCommandWordAndArgs(String rawUserInput) {
final String[] split = rawUserInput.trim().split("\\s+", 2);

return split.length == 2 ? split : new String[] { split[0] , "" }; // else case: no parameters
}

Expand Down Expand Up @@ -427,6 +456,7 @@ private static String executeAddPerson(String commandArgs) {
// add the person as specified
final String[] personToAdd = decodeResult.get();
addPersonToAddressBook(personToAdd);

return getMessageForSuccessfulAddPerson(personToAdd);
}

Expand All @@ -453,6 +483,7 @@ private static String executeFindPersons(String commandArgs) {
final Set<String> keywords = extractKeywordsFromFindPersonArgs(commandArgs);
final ArrayList<String[]> personsFound = getPersonsWithNameContainingAnyKeyword(keywords);
showToUser(personsFound);

return getMessageForPersonsDisplayedSummary(personsFound);
}

Expand Down Expand Up @@ -490,6 +521,7 @@ private static ArrayList<String[]> getPersonsWithNameContainingAnyKeyword(Collec
matchedPersons.add(person);
}
}

return matchedPersons;
}

Expand All @@ -503,11 +535,14 @@ private static String executeDeletePerson(String commandArgs) {
if (!isDeletePersonArgsValid(commandArgs)) {
return getMessageForInvalidCommandInput(COMMAND_DELETE_WORD, getUsageInfoForDeleteCommand());
}

final int targetVisibleIndex = extractTargetIndexFromDeletePersonArgs(commandArgs);
if (!isDisplayIndexValidForLastPersonListingView(targetVisibleIndex)) {
return MESSAGE_INVALID_PERSON_DISPLAYED_INDEX;
}

final String[] targetInModel = getPersonByLastVisibleIndex(targetVisibleIndex);

return deletePersonFromAddressBook(targetInModel) ? getMessageForSuccessfulDelete(targetInModel) // success
: MESSAGE_PERSON_NOT_IN_ADDRESSBOOK; // not found
}
Expand All @@ -521,6 +556,7 @@ private static String executeDeletePerson(String commandArgs) {
private static boolean isDeletePersonArgsValid(String rawArgs) {
try {
final int extractedIndex = Integer.parseInt(rawArgs.trim()); // use standard libraries to parse

return extractedIndex >= DISPLAYED_INDEX_OFFSET;
} catch (NumberFormatException nfe) {
return false;
Expand Down Expand Up @@ -565,6 +601,7 @@ private static String getMessageForSuccessfulDelete(String[] deletedPerson) {
*/
private static String executeClearAddressBook() {
clearAddressBook();

return MESSAGE_ADDRESSBOOK_CLEARED;
}

Expand All @@ -576,6 +613,7 @@ private static String executeClearAddressBook() {
private static String executeListAllPersonsInAddressBook() {
ArrayList<String[]> toBeDisplayed = getAllPersonsInAddressBook();
showToUser(toBeDisplayed);

return getMessageForPersonsDisplayedSummary(toBeDisplayed);
}

Expand All @@ -601,10 +639,12 @@ private static void executeExitProgramRequest() {
private static String getUserInput() {
System.out.print(LINE_PREFIX + "Enter command: ");
String inputLine = SCANNER.nextLine();

// silently consume all blank and comment lines
while (inputLine.trim().isEmpty() || inputLine.trim().charAt(0) == INPUT_COMMENT_MARKER) {
inputLine = SCANNER.nextLine();
}

return inputLine;
}

Expand Down Expand Up @@ -647,6 +687,7 @@ private static String getDisplayString(ArrayList<String[]> persons) {
.append(getIndexedPersonListElementMessage(displayIndex, person))
.append(LS);
}

return messageAccumulator.toString();
}

Expand Down Expand Up @@ -706,6 +747,7 @@ private static String[] getPersonByLastVisibleIndex(int lastVisibleIndex) {
*/
private static void createFileIfMissing(String filePath) {
final File storageFile = new File(filePath);

if (storageFile.exists()) {
return;
}
Expand Down Expand Up @@ -734,6 +776,7 @@ private static ArrayList<String[]> loadPersonsFromFile(String filePath) {
showToUser(MESSAGE_INVALID_STORAGE_FILE_CONTENT);
exitProgram();
}

return successfullyDecoded.get();
}

Expand All @@ -752,6 +795,7 @@ private static ArrayList<String> getLinesInFile(String filePath) {
showToUser(String.format(MESSAGE_ERROR_READING_FROM_FILE, filePath));
exitProgram();
}

return lines;
}

Expand Down Expand Up @@ -798,6 +842,7 @@ private static boolean deletePersonFromAddressBook(String[] exactPerson) {
if (changed) {
savePersonsToFile(getAllPersonsInAddressBook(), storageFilePath);
}

return changed;
}

Expand Down Expand Up @@ -870,9 +915,11 @@ private static String getEmailFromPerson(String[] person) {
*/
private static String[] makePersonFromData(String name, String phone, String email) {
final String[] person = new String[PERSON_DATA_COUNT];

person[PERSON_DATA_INDEX_NAME] = name;
person[PERSON_DATA_INDEX_PHONE] = phone;
person[PERSON_DATA_INDEX_EMAIL] = email;

return person;
}

Expand All @@ -898,6 +945,7 @@ private static ArrayList<String> encodePersonsToStrings(ArrayList<String[]> pers
for (String[] person : persons) {
encoded.add(encodePersonToString(person));
}

return encoded;
}

Expand All @@ -920,11 +968,13 @@ private static Optional<String[]> decodePersonFromString(String encoded) {
if (!isPersonDataExtractableFrom(encoded)) {
return Optional.empty();
}

final String[] decodedPerson = makePersonFromData(
extractNameFromPersonString(encoded),
extractPhoneFromPersonString(encoded),
extractEmailFromPersonString(encoded)
);

// check that the constructed person is valid
return isPersonDataValid(decodedPerson) ? Optional.of(decodedPerson) : Optional.empty();
}
Expand All @@ -945,6 +995,7 @@ private static Optional<ArrayList<String[]>> decodePersonsFromStrings(ArrayList<
}
decodedPersons.add(decodedPerson.get());
}

return Optional.of(decodedPersons);
}

Expand All @@ -957,6 +1008,7 @@ private static Optional<ArrayList<String[]>> 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()
Expand All @@ -972,8 +1024,10 @@ private static boolean isPersonDataExtractableFrom(String personData) {
private static String extractNameFromPersonString(String encoded) {
final int indexOfPhonePrefix = encoded.indexOf(PERSON_DATA_PREFIX_PHONE);
final int indexOfEmailPrefix = encoded.indexOf(PERSON_DATA_PREFIX_EMAIL);

// name is leading substring up to first data prefix symbol
int indexOfFirstPrefix = Math.min(indexOfEmailPrefix, indexOfPhonePrefix);

return encoded.substring(0, indexOfFirstPrefix).trim();
}

Expand All @@ -989,12 +1043,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 +1066,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 @@ -1150,8 +1204,8 @@ private static String getUsageInfoForExitCommand() {
* @param sign Parameter sign to be removed
* @return string without the sign
Copy link

Choose a reason for hiding this comment

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

need to update the header comments after updating the method.

*/
private static String removePrefixSign(String s, String sign) {
return s.replace(sign, "");
private static String removePrefix(String fullString, String prefix) {
return fullString.replace(prefix, "");
Copy link

Choose a reason for hiding this comment

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

as mentioned in tutorial, the replace behaviour would also remove instances of the prefix argument that are found else where than the start of fullString

}

/**
Expand Down