-
Notifications
You must be signed in to change notification settings - Fork 166
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.7][T16-4]Rezky Arizaputra #427
base: master
Are you sure you want to change the base?
Changes from all commits
43833c2
bd1c66a
cad5ad4
56f9395
1c50e62
5a25056
08f39c1
d4d1784
4d999c1
c1aa1a8
b075724
2d003f3
1abe739
64f2035
921fb0b
63490be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package seedu.addressbook.commands; | ||
|
||
/** | ||
* Sort the list of people in address book | ||
*/ | ||
public class SortCommand extends Command { | ||
public static final String COMMAND_WORD = "sort"; | ||
public static final String MESSAGE_USAGE = COMMAND_WORD | ||
+ "Sorts all people in address book. \n" | ||
+ "Example: " + COMMAND_WORD; | ||
|
||
public static final String MESSAGE_SORTED = "Address book is sorted"; | ||
@Override | ||
public CommandResult execute() { | ||
addressBook.sort(); | ||
return new CommandResult(MESSAGE_SORTED); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
package seedu.addressbook.commands; | ||
|
||
import org.junit.Before; | ||
import org.junit.Test; | ||
import seedu.addressbook.data.AddressBook; | ||
import seedu.addressbook.data.person.*; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid using |
||
import seedu.addressbook.util.TestUtil; | ||
|
||
import java.util.Collections; | ||
import java.util.List; | ||
|
||
import static org.junit.Assert.*; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no space between this import line and the |
||
public class SortCommandTest { | ||
|
||
private AddressBook addressBook; | ||
|
||
private List<ReadOnlyPerson> sortedList; | ||
|
||
public void setUp() throws Exception { | ||
Person SamAl = new Person(new Name("Sam Al"), new Phone("65234567", false), | ||
new Email("[email protected]", false), new Address("395C Bukit Road", false), Collections.emptySet()); | ||
Person SamUl = new Person(new Name("Sam Ul"), new Phone("95234567", false), | ||
new Email("[email protected]", false), new Address("33G Bata Road", false), Collections.emptySet()); | ||
Person SamIl = new Person(new Name("Sam Il"), new Phone("65345566", false), | ||
new Email("[email protected]", false), new Address("55G Paku Road", false), Collections.emptySet()); | ||
Person SamEl = new Person(new Name("Sam El"), new Phone("65121122", false), | ||
new Email("[email protected]", false), new Address("44H Buana Road", false), | ||
Collections.emptySet()); | ||
|
||
addressBook = TestUtil.createAddressBook(SamAl, SamUl, SamIl, SamEl); | ||
|
||
|
||
sortedList = TestUtil.createList(SamAl, SamEl, SamIl, SamUl); | ||
} | ||
|
||
@Test | ||
public void execute_addressBookIsSorted() { | ||
assertSortSuccessful(addressBook, sortedList); | ||
} | ||
|
||
/** | ||
* Creates a new sorting command. | ||
* | ||
*/ | ||
private SortingCommand createSortingCommand(AddressBook addressBook, | ||
List<ReadOnlyPerson> displayList) { | ||
|
||
SortingCommand command = new SortingCommand(); | ||
command.setData(addressBook, displayList); | ||
|
||
return command; | ||
} | ||
/** | ||
* Asserts that the address book is successfully sorted. | ||
* | ||
* The addressBook passed in will not be modified (no side effects). | ||
*/ | ||
private void assertSortingSuccessful(AddressBook addressBook, | ||
List<ReadOnlyPerson> displayList) { | ||
AddressBook expectedAddressBook = TestUtil.clone(addressBook); | ||
expectedAddressBook.sort(); | ||
String expectedMessage = SortingCommand.MESSAGE_SORTED; | ||
|
||
AddressBook actualAddressBook = TestUtil.clone(addressBook); | ||
|
||
SortingCommand command = createSortingCommand(actualAddressBook, displayList); | ||
assertCommandBehaviour(command, expectedMessage, expectedAddressBook, actualAddressBook); | ||
|
||
} | ||
|
||
/** | ||
* Executes the command, and checks that the execution was what we had expected. | ||
*/ | ||
private void assertCommandBehaviour(SortingCommand sortCommand, String expectedMessage, | ||
AddressBook expectedAddressBook, AddressBook actualAddressBook) { | ||
|
||
CommandResult result = sortCommand.execute(); | ||
|
||
assertEquals(expectedMessage, result.feedbackToUser); | ||
assertEquals(expectedAddressBook.getAllPersons(), actualAddressBook.getAllPersons()); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think might be good to not have so many blank lines at the end of the class. Also, |
||
|
||
|
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package seedu.addressbook.data.person; | ||
|
||
import java.util.Comparator; | ||
|
||
class CompareName implements Comparator<Person>{ | ||
@Override | ||
public int compare(Person p1, Person p2) { | ||
return p1.getName().toString() | ||
.compareToIgnoreCase(p2.getName().toString()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,6 +128,11 @@ public void remove(ReadOnlyPerson toRemove) throws PersonNotFoundException { | |
public void clear() { | ||
internalList.clear(); | ||
} | ||
|
||
/** | ||
* Sorts all people in the list. | ||
*/ | ||
public void sort() {Collections.sort(internalList, new CompareName()); } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no space after |
||
|
||
@Override | ||
public Iterator<Person> iterator() { | ||
|
@@ -141,3 +146,5 @@ public boolean equals(Object other) { | |
&& this.internalList.equals(((UniquePersonList) other).internalList)); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation for this method is inconsistent with the lines before it. Java unlike languages like Ruby or Python compiles fine with inconsistent indentation. But it is still important to have consistent proper indentation to make it easy for the user to read and mke your code neat in general