Merge pull request #147 from AbdulrahmanAlRammah/Sort_Improvements

Sort improvements

docs/UserGuide.md

@@ -196,12 +196,12 @@ Sorts the list of persons being viewed by name or date of last visit in ascendin
 Format: `sort parameter/order`
 
 * Sorts the displayed list of persons according to the specified order.
-* Order can be specified as ascending by leaving the order blank or **asc**/**ascending**
-* Order can be specified as descending by **descending**/**desc**
+* Order can be specified as ascending by leaving the order blank or **a**/**asc**/**ascend**/**ascending**
+* Order can be specified as descending by **d**/**desc**/**descend**/**descending**
 
 Examples:
 * `sort n/` sorts by name in ascending order.
-* `sort d/descending` sorts by date of last visit in descending order.
+* `sort d/desc` sorts by date of last visit in descending order.
 
 ### Clearing all entries : `clear`
 

src/main/java/seedu/address/logic/parser/SortCommandParser.java

@@ -9,15 +9,20 @@
 import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;
 
+import java.util.Set;
+
 import seedu.address.logic.commands.SortCommand;
 import seedu.address.logic.parser.exceptions.ParseException;
 import seedu.address.model.person.PersonComparator;
 
+
 /**
  * Parses input arguments and creates a new SortCommand Object.
  */
 public class SortCommandParser implements Parser<SortCommand> {
     private static final Prefix[] INVALID_PREFIXES = {PREFIX_ADDRESS, PREFIX_EMAIL, PREFIX_PHONE, PREFIX_TAG};
+    private static final Set<String> VALID_ASCEND_STRINGS = Set.of("", "a", "asc", "ascend", "ascending");
+    private static final Set<String> VALID_DESCEND_STRINGS = Set.of("d", "desc", "descend", "descending");
 
     /**
      * Checks if the given {@code String} of arguments is valid
@@ -52,13 +57,10 @@ public SortCommand parse(String args) throws ParseException {
         }
     }
 
-    private final boolean isAscending(String s) throws ParseException {
-        if (s.isEmpty()) {
-            return true;
-        }
-        if (s.equals("asc") || s.equals("ascending")) {
+    private boolean isAscending(String s) throws ParseException {
+        if (VALID_ASCEND_STRINGS.contains(s)) {
             return true;
-        } else if (s.equals("desc") || s.equals("descending")) {
+        } else if (VALID_DESCEND_STRINGS.contains(s)) {
             return false;
         }
         throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, SortCommand.MESSAGE_USAGE));

src/main/java/seedu/address/model/person/PersonComparator.java

@@ -10,7 +10,7 @@
  */
 public class PersonComparator {
     public static final String NAME = "name";
-    public static final String DATE_OF_LAST_VISIT = "dateoflastvisit";
+    public static final String DATE_OF_LAST_VISIT = "date of last visit";
     public static final String EARLIEST_VALID_DATE = "01-01-0001";
     public static final String LATEST_VALID_DATE = "31-12-9999";
     private static final String SORT_EXCEPTION = "The specified parameter is invalid.";

src/test/java/seedu/address/logic/commands/SortCommandTest.java

@@ -24,45 +24,29 @@ public class SortCommandTest {
 
     @Test
     public void execute_nameAscendingOrder_success() {
-        SortCommand sortCommand = new SortCommand(PersonComparator.NAME, true);
-        String expectedMessage = String.format(SortCommand.MESSAGE_SUCCESS,
-                PersonComparator.NAME, "ascending");
-        Model expectedModel = new ModelManager(getTypicalAddressBook(PersonComparator
-                .NAME, true), new UserPrefs());
-
-        assertCommandSuccess(sortCommand, modelDescendingName, expectedMessage, expectedModel);
+        executeSuccessfulSortTest(PersonComparator.NAME, true, modelAscendingDateOfLastVisit);
     }
 
     @Test
     public void execute_nameDescendingOrder_success() {
-        SortCommand sortCommand = new SortCommand(PersonComparator.NAME, false);
-        String expectedMessage = String.format(SortCommand.MESSAGE_SUCCESS,
-                PersonComparator.NAME, "descending");
-        Model expectedModel = new ModelManager(getTypicalAddressBook(PersonComparator
-                .NAME, false), new UserPrefs());
-
-        assertCommandSuccess(sortCommand, modelAscendingName, expectedMessage, expectedModel);
+        executeSuccessfulSortTest(PersonComparator.NAME, false, modelDescendingDateOfLastVisit);
     }
 
     @Test
     public void execute_dateOfLastVisitAscending_success() {
-        SortCommand sortCommand = new SortCommand(PersonComparator.DATE_OF_LAST_VISIT, true);
-        String expectedMessage = String.format(SortCommand.MESSAGE_SUCCESS,
-                PersonComparator.DATE_OF_LAST_VISIT, "ascending");
-        Model expectedModel = new ModelManager(getTypicalAddressBook(PersonComparator
-                .DATE_OF_LAST_VISIT, true), new UserPrefs());
-
-        assertCommandSuccess(sortCommand, modelDescendingName, expectedMessage, expectedModel);
+        executeSuccessfulSortTest(PersonComparator.DATE_OF_LAST_VISIT, true, modelDescendingName);
     }
 
     @Test
     public void execute_dateOfLastVisitDescending_success() {
-        SortCommand sortCommand = new SortCommand(PersonComparator.DATE_OF_LAST_VISIT, false);
-        String expectedMessage = String.format(SortCommand.MESSAGE_SUCCESS,
-                PersonComparator.DATE_OF_LAST_VISIT, "descending");
-        Model expectedModel = new ModelManager(getTypicalAddressBook(PersonComparator
-                .DATE_OF_LAST_VISIT, false), new UserPrefs());
+        executeSuccessfulSortTest(PersonComparator.DATE_OF_LAST_VISIT, false, modelAscendingName);
+    }
 
+    private void executeSuccessfulSortTest(String sortParameter, boolean isAscending, Model model) {
+        SortCommand sortCommand = new SortCommand(sortParameter, isAscending);
+        String expectedMessage = String.format(SortCommand.MESSAGE_SUCCESS,
+                sortParameter, isAscending ? "ascending" : "descending");
+        Model expectedModel = new ModelManager(getTypicalAddressBook(sortParameter, isAscending), new UserPrefs());
         assertCommandSuccess(sortCommand, modelDescendingName, expectedMessage, expectedModel);
     }
 

src/test/java/seedu/address/logic/parser/SortCommandParserTest.java

@@ -55,16 +55,19 @@ public void parse_validArgs_success() {
         SortCommand descendingDateOfLastVisitSortCommand =
                 new SortCommand(PersonComparator.DATE_OF_LAST_VISIT, false);
 
+
         assertParseSuccess(parser, " n/ ", ascendingNameSortCommand);
         assertParseSuccess(parser, " n/ascending   ", ascendingNameSortCommand);
         assertParseSuccess(parser, " n/asc", ascendingNameSortCommand);
 
         assertParseSuccess(parser, " d/", ascendingDateOfLastVisitSortCommand);
         assertParseSuccess(parser, " d/ascending   ", ascendingDateOfLastVisitSortCommand);
-        assertParseSuccess(parser, " d/asc", ascendingDateOfLastVisitSortCommand);
+        assertParseSuccess(parser, " d/   asc", ascendingDateOfLastVisitSortCommand);
+        assertParseSuccess(parser, " d/a", ascendingDateOfLastVisitSortCommand);
 
         assertParseSuccess(parser, " n/descending", descendingNameSortCommand);
-        assertParseSuccess(parser, " n/desc", descendingNameSortCommand);
+        assertParseSuccess(parser, " n/  desc", descendingNameSortCommand);
+        assertParseSuccess(parser, " n/d", descendingNameSortCommand);
 
         assertParseSuccess(parser, " d/descending", descendingDateOfLastVisitSortCommand);
         assertParseSuccess(parser, " d/desc", descendingDateOfLastVisitSortCommand);