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

[W5.11r][F11-B1]Sheikh Umar #755

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
3060d8d
I updated the user guide to reflect the sort command.
Mar 4, 2018
f7fc7d3
I updated the help command by adding in the sort command.
Mar 4, 2018
5a4c737
I added a sort command.
Mar 4, 2018
1b1c595
no message
Mar 4, 2018
f3545b9
I added a test case for sort command.
Mar 4, 2018
8cc0449
no message
Mar 4, 2018
8f1c0c0
I updated the Sort Command message based on feedback from Travis.
Mar 4, 2018
0d13337
Updated UniquePersonList based on feedback from Travis.
Mar 4, 2018
7f8742b
I corrected a typo error.
Mar 4, 2018
f50cf24
I updated the user command for the edit command.
Mar 5, 2018
01997fb
I updated the EditCommand, HelpCommand, Messages, and Parser java files.
Mar 5, 2018
a8d5b7e
I updated the code based on feedback from Travis.
Mar 5, 2018
68db7a3
I made changes to the EditCommand and Parser java files based on feed…
Mar 5, 2018
29188e0
I stated the sources that I referred to for these particular java files.
Mar 5, 2018
40208ee
no message
Mar 5, 2018
733224c
I updated the find command of the User Guide.
Mar 5, 2018
1d6df7b
Revert "I updated the find command of the User Guide."
Mar 5, 2018
e14d382
Added a test case to StorageFileTest.java
Mar 5, 2018
9f7cb53
no message
Mar 5, 2018
ccb86ac
Merge branch 'W5.11r' of https://github.com/Sheikh-Umar/addressbook-l…
Mar 5, 2018
8ba31f1
I updated the StorageFileTest.java.
Mar 5, 2018
b96dc5c
I updated the add command to make the name entered to lowercase.
Mar 5, 2018
1b67d4d
Revert "I updated the add command to make the name entered to lowerca…
Mar 5, 2018
ee9d207
no message
Mar 5, 2018
db1d746
no message
Mar 5, 2018
81c8d72
Converts name to lowercase.
Mar 5, 2018
ecf7c7c
no message
Mar 5, 2018
b54df6c
Updated User Guide, Name & Parser documents for making add and find c…
Mar 8, 2018
ff9e06a
Merge commit 'ecf7c7ce71dd9c404c0bef9e6e872a202b35c8d9' into W5.11r
Mar 8, 2018
7edfb2a
no message
Mar 8, 2018
fa1ed45
Updated FindCommand and AddCommand.
Mar 8, 2018
339a1ec
Updated Clear Command.
Mar 8, 2018
98b9723
Updated Name java document.
Mar 8, 2018
2052e30
Updated Name java document.
Mar 8, 2018
4c5a231
Updated ClearCommand.
Mar 8, 2018
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
22 changes: 22 additions & 0 deletions doc/UserGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,24 @@ Examples:
`delete 1`<br>
Deletes the 1st person in the results of the `find` command.

## Editing a person: `edit`
Edits a particular attribute of a person in the address book. Irreversible.<br>
Format: `edit INDEX ATTRIBUTE NEW_VALUE`

> Edits the person at the specified `INDEX` by replacing the `ATTRIBUTE` of the person with the `NEW_VALUE`.
The index refers to the index number shown in the most recent listing.

> If an invalid `ATTRIBUTE` is entered or the `NEW_VALUE` that is entered is identical
to the current value of the `ATTRIBUTE`, there will be no changes to be made.

Examples:
* `list`<br>
`edit 2 phone 90091234`<br>
Edits the 2nd person in the address book by updating his/her phone number to "90091234".
`list`<br>
`edit 1 name Jake`<br>
Edits the 1st person in the address book by updating his/her name to "Jake".

## View non-private details of a person : `view`
Displays the non-private details of the specified person.<br>
Format: `view INDEX`
Expand Down Expand Up @@ -100,6 +118,10 @@ Examples:
Clears all entries from the address book.<br>
Format: `clear`

## Sorting all entries: `sort`
Sorts all entries in the address book in increasing alphabetical order based on the first name.<br>
Format: `sort`

## Exiting the program : `exit`
Exits the program.<br>
Format: `exit`
Expand Down
108 changes: 108 additions & 0 deletions src/seedu/addressbook/commands/EditCommand.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package seedu.addressbook.commands;

import seedu.addressbook.common.Messages;
import seedu.addressbook.data.exception.IllegalValueException;
import seedu.addressbook.data.person.Address;
import seedu.addressbook.data.person.Email;
import seedu.addressbook.data.person.Name;
import seedu.addressbook.data.person.Person;
import seedu.addressbook.data.person.Phone;
import seedu.addressbook.data.person.ReadOnlyPerson;
import seedu.addressbook.data.person.UniquePersonList.PersonNotFoundException;
import seedu.addressbook.parser.Parser.Attribute;

/**
* Edits the attribute of the person identified
* based on the last-displayed index of the person in the address book
* by updating his/her atttribute with the new value.
*/

//Solution below adapted from https://github.com/nus-cs2103-AY1718S2/addressbook-level3/pull/749/files
Copy link

Choose a reason for hiding this comment

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

We do not condone wholesale copying of enhancements for LO submissions. Do not do this again. Since you have the sort command, I will be only reviewing the sort command enhancement.

public class EditCommand extends Command {

public static final String COMMAND_WORD = "edit";

public static final String MESSAGE_USAGE = COMMAND_WORD
+ ": Edits an attribute of a person identified by an index number "
+ "used in the last find/last call.\n"
+ "Parameters: INDEX ATTRIBUTE NEW_VALUE\n"
+ "Example: " + COMMAND_WORD + "2 name Tom";

public static final String MESSAGE_EDIT_PERSON_SUCCESS = "Edit is successful!\n"
+ "Original Person: %1$s\n"
+ ", Edited Person: %2$s";

private final Attribute attribute;

private final String newValue;

public EditCommand(int targetIndex, Attribute attribute, String newValue) {
super(targetIndex);
this.attribute = attribute;
this.newValue = newValue;
}

@Override
public CommandResult execute() {

try {
final ReadOnlyPerson target = getTargetPerson();
if(isNewValueSameAsOld(target)) {
return new CommandResult(Messages.MESSAGE_INVALID_NEW_VALUE);
}
Person updatedPerson = updateExistingPerson(target);
addressBook.removePerson(target);
addressBook.addPerson(updatedPerson);
return new CommandResult(String.format(MESSAGE_EDIT_PERSON_SUCCESS,
target.getAsTextHidePrivate(),
updatedPerson.getAsTextHidePrivate()));
} catch(IndexOutOfBoundsException ie) {
return new CommandResult(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
} catch(PersonNotFoundException pnfe) {
return new CommandResult(Messages.MESSAGE_PERSON_NOT_IN_ADDRESSBOOK);
} catch(IllegalValueException ive) {
return new CommandResult(ive.getMessage());
}
}

private boolean isNewValueSameAsOld(ReadOnlyPerson target) {

switch(attribute) {
case ADDRESS:
return target.getAddress().toString().equals(newValue);
case EMAIL:
return target.getEmail().toString().equals(newValue);
case NAME:
return target.getName().toString().equals(newValue);
case PHONE:
return target.getPhone().toString().equals(newValue);
default:
return true;
}
}

private Person updateExistingPerson(ReadOnlyPerson existingPerson) throws IllegalValueException {
Address address = existingPerson.getAddress();
Email email = existingPerson.getEmail();
Name name = existingPerson.getName();
Phone phone = existingPerson.getPhone();

switch(attribute) {
case ADDRESS:
address = new Address(newValue, address.isPrivate());
break;
case EMAIL:
email = new Email(newValue, email.isPrivate());
break;
case NAME:
name = new Name(newValue);
break;
case PHONE:
phone = new Phone(newValue, phone.isPrivate());
break;
}

return new Person(name, phone, email, address, existingPerson.getTags());
}

}
4 changes: 3 additions & 1 deletion src/seedu/addressbook/commands/HelpCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ public class HelpCommand extends Command {
public static final String MESSAGE_ALL_USAGES = AddCommand.MESSAGE_USAGE
+ "\n" + DeleteCommand.MESSAGE_USAGE
+ "\n" + ClearCommand.MESSAGE_USAGE
+ "\n" + EditCommand.MESSAGE_USAGE
+ "\n" + FindCommand.MESSAGE_USAGE
+ "\n" + ListCommand.MESSAGE_USAGE
+ "\n" + ViewCommand.MESSAGE_USAGE
+ "\n" + ViewAllCommand.MESSAGE_USAGE
+ "\n" + HelpCommand.MESSAGE_USAGE
+ "\n" + ExitCommand.MESSAGE_USAGE;
+ "\n" + ExitCommand.MESSAGE_USAGE
+ "\n" + SortCommand.MESSAGE_USAGE;

@Override
public CommandResult execute() {
Expand Down
25 changes: 25 additions & 0 deletions src/seedu/addressbook/commands/SortCommand.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package seedu.addressbook.commands;

import seedu.addressbook.data.person.Person;

import java.util.List;

/**
* Lists all persons in the address book in a sorted order based on the first name of the person
*/
public class SortCommand extends Command {

public static final String COMMAND_WORD = "sort";

public static final String MESSAGE_USAGE = COMMAND_WORD + ":\n" + "Lists all persons in the address book in a sorted order "
+ "based on the first name of the person with index numbers.\n\t"
+ "Example: " + COMMAND_WORD;

@Override
public CommandResult execute() {
List<Person> allPersons = addressBook.getAllPersons().getInternalList();
allPersons.sort((Person p1, Person p2) -> (p1.getName().toString()).compareTo(p2.getName().toString()));
return new CommandResult(getMessageForPersonListShownSummary(allPersons), allPersons);
}

}
2 changes: 2 additions & 0 deletions src/seedu/addressbook/common/Messages.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ public class Messages {
public static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format! \n%1$s";
public static final String MESSAGE_INVALID_PERSON_DISPLAYED_INDEX = "The person index provided is invalid";
public static final String MESSAGE_PERSON_NOT_IN_ADDRESSBOOK = "Person could not be found in address book";
public static final String MESSAGE_INVALID_ATTRIBUTE = "Invalid attribute entered";
public static final String MESSAGE_INVALID_NEW_VALUE = "Invalid new value entered as it is the same as the old value";
public static final String MESSAGE_PERSONS_LISTED_OVERVIEW = "%1$d persons listed!";
public static final String MESSAGE_PROGRAM_LAUNCH_ARGS_USAGE = "Launch command format: " +
"java seedu.addressbook.Main [STORAGE_FILE_PATH]";
Expand Down
2 changes: 1 addition & 1 deletion src/seedu/addressbook/data/person/Name.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class Name {
public final String fullName;

/**
* Validates given name.
* Validates the given name.
*
* @throws IllegalValueException if given name string is invalid.
*/
Expand Down
4 changes: 4 additions & 0 deletions src/seedu/addressbook/data/person/UniquePersonList.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ public static class PersonNotFoundException extends Exception {}

private final List<Person> internalList = new ArrayList<>();

public List<Person> getInternalList() {
return internalList;
}

/**
* Constructs empty person list.
*/
Expand Down
51 changes: 49 additions & 2 deletions src/seedu/addressbook/parser/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,23 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static seedu.addressbook.common.Messages.MESSAGE_INVALID_ATTRIBUTE;
import static seedu.addressbook.common.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
import static seedu.addressbook.common.Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX;

/**
* Parses user input.
*/
//Solution below adapted from https://github.com/nus-cs2103-AY1718S2/addressbook-level3/pull/749/files
public class Parser {

public static final Pattern PERSON_INDEX_ARGS_FORMAT = Pattern.compile("(?<targetIndex>.+)");

public static final Pattern EDIT_PERSON_ARGS_FORMAT =
Pattern.compile("(?<targetIndex>\\S+)"
+ " (?<attribute>\\S+)"
+ " (?<newValue>\\S+)");

public static final Pattern KEYWORDS_ARGS_FORMAT =
Pattern.compile("(?<keywords>\\S+(?:\\s+\\S+)*)"); // one or more keywords separated by whitespace

Expand Down Expand Up @@ -63,6 +71,9 @@ public Command parseCommand(String userInput) {
case DeleteCommand.COMMAND_WORD:
return prepareDelete(arguments);

case EditCommand.COMMAND_WORD:
return prepareEdit(arguments);

case ClearCommand.COMMAND_WORD:
return new ClearCommand();

Expand All @@ -72,6 +83,9 @@ public Command parseCommand(String userInput) {
case ListCommand.COMMAND_WORD:
return new ListCommand();

case SortCommand.COMMAND_WORD:
return new SortCommand();

case ViewCommand.COMMAND_WORD:
return prepareView(arguments);

Expand Down Expand Up @@ -140,14 +154,14 @@ private static Set<String> getTagsFromArgs(String tagArguments) throws IllegalVa
return new HashSet<>(tagStrings);
}


/**
* Parses arguments in the context of the delete person command.
*
* @param args full command args string
* @return the prepared command
*/
private Command prepareDelete(String args) {

try {
final int targetIndex = parseArgsAsDisplayedIndex(args);
return new DeleteCommand(targetIndex);
Expand Down Expand Up @@ -214,7 +228,7 @@ private int parseArgsAsDisplayedIndex(String args) throws ParseException, Number
* @return the prepared command
*/
private Command prepareFind(String args) {
final Matcher matcher = KEYWORDS_ARGS_FORMAT.matcher(args.trim());
final Matcher matcher = KEYWORDS_ARGS_FORMAT.matcher(args.toLowerCase().trim());
if (!matcher.matches()) {
return new IncorrectCommand(String.format(MESSAGE_INVALID_COMMAND_FORMAT,
FindCommand.MESSAGE_USAGE));
Expand All @@ -226,5 +240,38 @@ private Command prepareFind(String args) {
return new FindCommand(keywordSet);
}

/**
* Parses arguments in the context of the editing person command.
*
* @param args full command args string
* @return the prepared command
*/
private Command prepareEdit(String args) {
final Matcher matcher = KEYWORDS_ARGS_FORMAT.matcher(args.trim());
// Validate arg string format
if (!matcher.matches()) {
return new IncorrectCommand(String.format(MESSAGE_INVALID_COMMAND_FORMAT,
EditCommand.MESSAGE_USAGE));
}
try {
String originalTargetIndex = matcher.group("targetIndex").trim();
String originalAttribute = matcher.group("attribute").trim();
final int targetIndex = parseArgsAsDisplayedIndex(originalTargetIndex);
final Attribute attribute = Attribute.valueOf(originalAttribute.toUpperCase());
final String newValue = matcher.group("newValue").trim();
return new EditCommand(targetIndex, attribute, newValue);
} catch (ParseException pe) {
return new IncorrectCommand(String.format(MESSAGE_INVALID_COMMAND_FORMAT,
EditCommand.MESSAGE_USAGE));
} catch (NumberFormatException nfe) {
return new IncorrectCommand(MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
} catch (IllegalArgumentException iae) {
return new IncorrectCommand(MESSAGE_INVALID_ATTRIBUTE);
}
}

public enum Attribute {
NAME, PHONE, EMAIL, ADDRESS
}

}
24 changes: 24 additions & 0 deletions test/java/seedu/addressbook/logic/LogicTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,30 @@ public void execute_list_showsAllPersons() throws Exception {
expectedList);
}

@Test
public void execute_sort_showsAllPerson() throws Exception {
// prepare expectations
TestDataHelper helper = new TestDataHelper();
Person person1 = helper.generatePersonWithName("John Tan");
Copy link

Choose a reason for hiding this comment

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

Might want to name these variables John, Ali, etc instead of person1 person2 so as to make the generation of expectedList and unorderedList more readable.

Person person2 = helper.generatePersonWithName("Ali Baba");
Person person3 = helper.generatePersonWithName("Jill Tan");
Person person4 = helper.generatePersonWithName("Bob Lee");

List<Person> expectedList = helper.generatePersonList(person2, person4, person3, person1);
List<Person> unorderedList = helper.generatePersonList(person1, person2, person3, person4);

AddressBook expectedAB = helper.generateAddressBook(unorderedList);
Copy link

Choose a reason for hiding this comment

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

Why is your expectedAB generated from the unorderedList? Should it not be from expectedList?
You might have meant unorderedAB instead?


//prepare address book state
helper.addToAddressBook(addressBook, unorderedList);
assertCommandBehavior("sort",
Command.getMessageForPersonListShownSummary(expectedList),
expectedAB,
true,
expectedList);

}

@Test
public void execute_view_invalidArgsFormat() throws Exception {
String expectedMessage = String.format(MESSAGE_INVALID_COMMAND_FORMAT, ViewCommand.MESSAGE_USAGE);
Expand Down