-
Notifications
You must be signed in to change notification settings - Fork 104
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.11][F10-3]Shreyas Kuthanoor Prakash #181
base: master
Are you sure you want to change the base?
[W5.11][F10-3]Shreyas Kuthanoor Prakash #181
Conversation
…_command Added confirm/cancel clear option to after clear command
Includes Clear/Cancel command prompt guide
Update UserGuide
Add delete_name command
Trying to merge
added total command and edited user guide
[W5.11][CS2113-AY1819S1-F10-3]Ong Wee Keong
Revert "[W5.11][CS2113-AY1819S1-F10-3]Ong Wee Keong"
[W 5.11][CS2113-AY1819S1-F10-3]Ong Wee Keong
…ressbook-level2 into feature/top_five
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.
Neat implementation of the feature and will documented. You can also add more tests for the command itself and update the help command. Remember to create clean PR next time.
*/ | ||
|
||
public class TopFiveCommand extends Command{ | ||
public static final String COMMAND_WORD = "topFive"; |
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.
maybe to keep the command word format consistent, you can use top_five instead of camelCase, or simply top might be more intuitive and user-friendly
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.
alternatively, you might want to allow the user to specify how many persons to list, so command like 'top 5'
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they should be imported explicitly (refer to style guide)
Add functionality to list the first five persons of the addressbook.