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][T08-4]Phan Duy Nhat Tan #49

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

Conversation

tantantan277
Copy link

No description provided.

@tantantan277 tantantan277 changed the title [W5][T08]Phan Duy Nhat Tan [W5][T08-4]Phan Duy Nhat Tan Feb 11, 2019
@nus-se-pr-bot nus-se-pr-bot requested a review from okkhoy February 11, 2019 09:08
Copy link

@shanseet shanseet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice command. Good job!

@weizhang05
Copy link

Useful command to check the size of the address book. Commits are separated clearly with specific purpose.

Copy link

@stephlewyh stephlewyh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Phan,

Good first attempt, having updated the user guide and enhancement. Note that you could have provided unit test for each new enhancement and supply header comments for your enhancement.

@@ -151,6 +151,11 @@ Views all details of the 2nd person in the address book.
`viewall 1` +
Views all details of the 1st person in the results of the `find` command.

== Show the number of person in the address book : `size`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the description for this feature? It is missing.

public static final String MESSAGE_USAGE = COMMAND_WORD + ": Show the size of the address book.\n"
+ "Example: " + COMMAND_WORD;

@Override

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing header comment. All non-trivial methods should have java doc format header comments.

/**
* Show the size of the address book
*/
public int size() {return allPersons.size();}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

method call of size() should be put on a separate line.

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.

6 participants