Skip to content
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

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

ShreyasKp
Copy link

Add functionality to list the first five persons of the addressbook.

iamputradanish and others added 30 commits September 6, 2018 14:39
…_command

Added confirm/cancel clear option to after clear command
Includes Clear/Cancel command prompt guide
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
Copy link

@fanyiii fanyiii left a 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";
Copy link

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

Copy link

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.*;
Copy link

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants