Skip to content

Conversation

@Kelly9373
Copy link

Add sort command. Use the idea from the instructions on the CS2103/T website "Add a SortCommand class but make it simply a copy of the the existing ListCommand. Direct the sort command to the new SortCommand."

Copy link

@wn wn left a comment

Choose a reason for hiding this comment

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

Don't add code that you haven't implement. Create an issue instead.

Copy link

@wn wn left a comment

Choose a reason for hiding this comment

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

Commit message isn't a sentence. Don't end with a period.

}

/**
* Returns an unmodifiable java List view with elements cast as immutable {@link ReadOnlyPerson}s.
Copy link

Choose a reason for hiding this comment

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

Use @return and @param.

+ "\n" + ViewAllCommand.MESSAGE_USAGE
+ "\n" + HelpCommand.MESSAGE_USAGE
+ "\n" + ExitCommand.MESSAGE_USAGE
+ "\n" + SortCommand.MESSAGE_USAGE
Copy link

Choose a reason for hiding this comment

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

/nit: Why is sort at the end?

public static final String COMMAND_WORD = "sort";

public static final String MESSAGE_USAGE = COMMAND_WORD + ": sort people in the address book.\n"
+ "Example: " + COMMAND_WORD;
Copy link

Choose a reason for hiding this comment

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

Inconsistant '+' alignment.

import seedu.addressbook.data.person.UniquePersonList;
import seedu.addressbook.data.tag.Tag;

public class SortCommand extends Command{
Copy link

Choose a reason for hiding this comment

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

/nit space before {

Copy link

@jlks96 jlks96 left a comment

Choose a reason for hiding this comment

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

Good job on attempting the LO. You seem to have difficulty implementing the tests for the new sort command. Do approach me if you need any help :)

}

/**
* Constructs a feedback message to summarise an operation that displayed a list of sorted persons.
Copy link

Choose a reason for hiding this comment

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

summarize (use American spelling), displays

* @param personsDisplayed used to generate summary
* @return summary message for persons displayed
*/
public static String getMessageForPersonSortShownSummary(List<? extends ReadOnlyPerson> personsDisplayed) {
Copy link

Choose a reason for hiding this comment

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

Hmm... method name is not that intuitive

* Any changes to the internal list/elements are immediately visible in the returned list.
*/
public List<ReadOnlyPerson> sortedListView() {
//internalList.sort(); <--how to make this line work instead of using Collections.sort()? :(
Copy link

Choose a reason for hiding this comment

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

https://docs.oracle.com/javase/8/docs/api/java/util/List.html
You have to pass in a comparator in this case. However, we usually just use Collections.sort/ Arrays.sort

return new SortCommand();

case HelpCommand.COMMAND_WORD: // Fallthrough

Copy link

Choose a reason for hiding this comment

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

Avoid adding unnecessary new 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.

4 participants