[W7][T11-1]Windrich #37
Conversation
…he full addressbook immediately after adding a new person. usage: add&list John p/123 e/[email protected] a/City
There was a problem hiding this comment.
Hi Windrich,
Good effort here!
(1) The enhancement logic works out well -e.g. preparing the person object at the constructor, to be added later.
(2) Good practice in updating the documentation.
(3) Provided adequate JUnit tests, with proper valid test inputs and assertions.
One thing you could be more aware of - header comments should be included in your code. Refer to the coding conventions guidelines.
You may close this PR after reviewing this comment.
| + "Example: " + COMMAND_WORD | ||
| + " John Doe p/98765432 e/[email protected] a/311, Clementi Ave 2, #02-25 t/friends t/owesMoney"; | ||
|
|
||
| // public static final String MESSAGE_SUCCESS = "New person added: %1$s"; |
There was a problem hiding this comment.
Avoid leaving behind commented out code for submission.
| return toAdd; | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
Missing header comment. All non-trivial methods should have java doc format header comments.
| @@ -119,6 +122,33 @@ private Command prepareAdd(String args){ | |||
| } | |||
| } | |||
|
|
|||
| private Command prepareAddList(String args) { | |||
There was a problem hiding this comment.
Missing header comment. All non-trivial methods should have java doc format header comments.
| @@ -245,6 +245,73 @@ public void addCommand_duplicateTags_merged() throws IllegalValueException { | |||
| assertEquals(result.getPerson(), testPerson); | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
Missing header comment. All non-trivial methods should have java doc format header comments.
| parseAndAssertIncorrectWithMessage(resultMessage, inputs); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Missing header comment. All non-trivial methods should have java doc format header comments.
| assertEquals(result.getPerson(), testPerson); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Missing header comment. All non-trivial methods should have java doc format header comments.
| input += " t/" + tag.tagName; | ||
| } | ||
|
|
||
| final AddListCommand result = parseAndAssertCommandType(input, AddListCommand.class); |
There was a problem hiding this comment.
Missing header comment. All non-trivial methods should have java doc format header comments.
| @@ -271,6 +338,18 @@ private static String convertPersonToAddCommandString(ReadOnlyPerson person) { | |||
| return addCommand; | |||
| } | |||
|
|
|||
| private static String convertPersonToAddListCommandString(ReadOnlyPerson person) { | |||
There was a problem hiding this comment.
Missing header comment. All non-trivial methods should have java doc format header comments.
|
|
||
| /* | ||
| Add a person to the addressbook and list down everyone in the addressbook right after | ||
| */ |
There was a problem hiding this comment.
You could have named the command as AddPersonCommand instead of AddListCommand. Add list may imply adding a list of things to something else, which is not what you are doing here.
| addressBook.addPerson(toAdd); | ||
| List<ReadOnlyPerson> allPersons = addressBook.getAllPersons().immutableListView(); | ||
| return new CommandResult(getMessageForPersonListShownSummary(allPersons), allPersons); | ||
| // return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd)); |
There was a problem hiding this comment.
Again, avoid leaving behind commented out code for submission. This reduces code readability.
Added enhancement to addressbook-level3