Skip to content

Conversation

@ZZG229
Copy link

@ZZG229 ZZG229 commented Sep 12, 2018

add sort command

Copy link

@eugenepeh eugenepeh 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 completing the activity.
Some comments added. Please close the PR after reading the comments.

import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.*;

Choose a reason for hiding this comment

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

remember to disable intellij auto import wildcard
Why is wild card import bad?


@Override
public CommandResult execute() {
Comparator<? super ReadOnlyPerson> comparator = new Comparator<ReadOnlyPerson>() {

Choose a reason for hiding this comment

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

Good! another way would be to simplify it to use lambda expression.

@Test
public void parse_SortCommand_parsedCorrectly() {
final String input = "sort";
parseAndAssertCommandType(input, SortCommand.class);

Choose a reason for hiding this comment

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

good, but perhaps you can add a test to verify that your sort command is working properly?

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.

3 participants