[W5][W17-1] Lee Wei Kang #144
Conversation
| import seedu.addressbook.commands.ListCommand; | ||
| import seedu.addressbook.commands.ViewAllCommand; | ||
| import seedu.addressbook.commands.ViewCommand; | ||
| import seedu.addressbook.commands.*; |
There was a problem hiding this comment.
Suggest switching back to importing commands one by one as mentioned in class.
There was a problem hiding this comment.
Since you've added a new command, it would be good if you could add it in the Help Command's guide as instructions for new users
There was a problem hiding this comment.
Yes definitely! Thank you for your suggestion! (:
| == Listing all persons sorted by their names : `sort` | ||
|
|
||
| Shows a list of all persons sorted by their names, along with their non-private details, in the address book. + | ||
| Format: `list` |
There was a problem hiding this comment.
I think perhaps the format is wrong? For a sort command, the format should be: 'sort'?
There was a problem hiding this comment.
Yes thanks for pointing that out!
| import java.util.Collections; | ||
| import java.util.List; | ||
|
|
||
| public class SortCommand extends Command{ |
There was a problem hiding this comment.
Do look out for the missing whitespace between Command and {, and the javadoc comment for the class.
| private List<ReadOnlyPerson> listWithInorderedTypicalPersons = Arrays.asList(td.amy, td.candy, td.bill, td.dan); | ||
|
|
||
| @Test | ||
| public void execute_invalidIndex_returnsInvalidIndexMessage() { |
There was a problem hiding this comment.
The naming of this method seems a little unclear - are you testing for an invalid index?
There was a problem hiding this comment.
Hi Wei Kang, some comments:
- Good that you updated the User Guide, I/O tests and JUnit Tests.
- Look out for the minor coding style aspects (E.g. missing whitespaces)
- Consider all parts of the project that will need to be updated following your change. (E.g. Adding your command information to the
helpsection. - Think about ways in which you can improve your JUnit test. (E.g. what is the expected behaviour when the addressbook is empty?)
- You can also explore the use of the
setUp()method in JUnit for the setup involved in your tests (Check out theDeleteCommandTestclass in AB2). - Good that you used a branch to implement this enhancement.
Added sort command