-
Notifications
You must be signed in to change notification settings - Fork 113
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][T12-3]Tan Kian Wei Jason #167
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very handy command. Code is concise and coding style is coherent with the rest of the files. Perhaps include the option to sort by different comparators, like last name, first name, email address, etc., instead of just the default full name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Useful addition to the addressbook. Able to understand the code written clearly as it follows the same coding style. You could implement a way to enable the user to sort the list automatically whenever an additional input have been made for the next time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on your first PR! Please close the PR after you have read my comments.
@@ -72,6 +72,11 @@ Examples: | |||
Shows a list of all persons, along with their non-private details, in the address book. + | |||
Format: `list` | |||
|
|||
== Listing all persons : `sort` | |||
|
|||
Sorts the address book in alphabetical order. + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is unclear which field of the address book is being sorted.
@@ -73,33 +74,36 @@ public Command parseCommand(String userInput) { | |||
|
|||
switch (commandWord) { | |||
|
|||
case AddCommand.COMMAND_WORD: | |||
return prepareAdd(arguments); | |||
case AddCommand.COMMAND_WORD: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there should not be indentation for case clauses.
/** | ||
* Asserts that the addressbook has been successfully sorted. | ||
* | ||
* The addressBook passed in will not be modified (no side effects). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that sorting the address book does not result in a different ordering of the persons?
Added sort command which sorts the addressbook in alphabetical order.
Also updated tests and user documentation to include sort command.