Skip to content

[W5][T09-2]Wan Chunwei - #35

Open
Wanchunwei wants to merge 5 commits into
nusCS2113-AY1819S2:masterfrom
Wanchunwei:W5.1
Open

[W5][T09-2]Wan Chunwei#35
Wanchunwei wants to merge 5 commits into
nusCS2113-AY1819S2:masterfrom
Wanchunwei:W5.1

Conversation

@Wanchunwei

Copy link
Copy Markdown

Added a sort command to sort all people by their name.

@BrittonAlone

Copy link
Copy Markdown

A simple but useful feature. We can improve it into sorting by date. Then it can be used in our project.

@ghost

ghost commented Feb 10, 2019

Copy link
Copy Markdown

it's very thoughtful to add the message for invalid command.

@sanjukta99 sanjukta99 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good job! Please remember to update the User Guide to reflect the new command that you have created. Please read the reviews and close this PR.

import seedu.addressbook.data.person.UniquePersonList.DuplicatePersonException;

public class SortCommand extends Command{
public static final String COMMAND_WORD = "Sort";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could add a header comment that explains what the Sort Command does to the user.

Comment thread test/input.txt
add Charlie Dickson pp/333333 e/charlie.d@nus.edu.sg a/333, gamma street t/friends t/school
list
sort

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good job on updating the test cases!

return 1;
}
else if(this.fullName.compareTo(o.fullName) < 0) {
return -1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please follow the Java Coding Standards which can be found on the module website for the correct format for if-else statements.

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.

5 participants