[W7][T11-2]WU PEI HSUAN #30
Conversation
There was a problem hiding this comment.
Hi Cherrie,
Good effort here. You have done well in making the enhancement work, providing a unit test and updating the user guide accordingly.
Some points you could improve on:
(1) Supplementing test cases and a JUnit test for the actual sorting that your enhancement is doing. Checking that the command is typed correctly is insufficient.
(2) Empty lines - You are leaning toward violating the coding convention for inserting too many empty lines in your code.
You may close this PR after reviewing this comment.
| + "Example: " + COMMAND_WORD; | ||
|
|
||
|
|
||
| @Override |
There was a problem hiding this comment.
Missing header comment. All non-trivial methods should have java doc format header comments.
| @Override | ||
| public CommandResult execute() { | ||
| addressBook.sorted(); | ||
|
|
There was a problem hiding this comment.
Be aware that too many empty lines can be a coding violation.
| * Sorts all persons from the address book. | ||
| */ | ||
|
|
||
| public void sorted() { allPersons.sort();} |
There was a problem hiding this comment.
The placement of your code is violating the coding convention. Please add logic on new line. Refer to java coding conventions for CS2113/T for more information.
| @@ -106,13 +106,27 @@ public void remove(ReadOnlyPerson toRemove) throws PersonNotFoundException { | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Again, we consider too many empty lines as a coding violation, as it reduces code readability.
| @@ -58,6 +58,12 @@ public void listCommand_parsedCorrectly() { | |||
| parseAndAssertCommandType(input, ListCommand.class); | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
Missing header comment. All non-trivial methods should have java doc format header comments.
No description provided.