Bug fix: Wrong error message in CommonFreeTimeCommand

Wrong error thrown for out of bounds index with CFT command. Error thrown was caught by an error catcher and a different error was thrown. Take out the error throwing from the try catch environment. Fix testcases affected by this bug fix.
AY2324S1-CS2103T-W12-4 · Nov 9, 2023 · 52e21bb · 52e21bb
1 parent 9166bbb
commit 52e21bb
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 40 deletions.
diff --git a/src/main/java/seedu/address/logic/commands/CommonFreetimeCommand.java b/src/main/java/seedu/address/logic/commands/CommonFreetimeCommand.java
@@ -70,12 +70,11 @@ public CommandResult execute(Model model) throws CommandException {
                 return new CommandResult(createCommonFreeTimeMessage(overlappingContacts, user).toString());
             }
         } else {
+            List<Person> lastShownList = model.getFilteredPersonList();
+            if (index.getZeroBased() >= lastShownList.size()) {
+                throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
+            }
             try {
-                List<Person> lastShownList = model.getFilteredPersonList();
-                if (index.getZeroBased() >= lastShownList.size()) {
-                    throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
-                }
-
                 Person friend = lastShownList.get(index.getZeroBased());
 
                 return new CommandResult(createCommonFreeTimeMessage(user, friend).toString());

diff --git a/src/main/java/seedu/address/logic/parser/AddressBookParser.java b/src/main/java/seedu/address/logic/parser/AddressBookParser.java
@@ -95,7 +95,7 @@ public Command parseCommand(String userInput) throws ParseException {
             return new RemoveReminderCommandParser().parse(arguments);
 
         case CommonFreetimeCommand.COMMAND_WORD:
-            return new CommonFreetimeCommandParser().parse(arguments);
+            return new CommonFreeTimeCommandParser().parse(arguments);
 
         case AddEventCommand.COMMAND_WORD:
             return new AddEventCommandParser().parse(arguments);

diff --git a/...c/parser/CommonFreetimeCommandParser.java → ...c/parser/CommonFreeTimeCommandParser.java b/...c/parser/CommonFreetimeCommandParser.java → ...c/parser/CommonFreeTimeCommandParser.java
@@ -11,7 +11,7 @@
 /**
  * Parses input arguments and creates a new CommonFreetimeCommand object.
  */
-public class CommonFreetimeCommandParser implements Parser<CommonFreetimeCommand> {
+public class CommonFreeTimeCommandParser implements Parser<CommonFreetimeCommand> {
 
     /**
      * Parses the given {@code String} of arguments in the context of the CommonFreetimeCommand

diff --git a/src/main/java/seedu/address/model/person/timetable/Schedule.java b/src/main/java/seedu/address/model/person/timetable/Schedule.java
@@ -260,7 +260,8 @@ public void addModule(String moduleString) throws IllegalValueException {
         if (!isOverlapping(newModule)) {
             modulesList.add(newModule);
         } else {
-            throw new IllegalArgumentException("Module " + newModule.getName() + " overlaps with " + getOverlappingEvent(newModule) + "!");
+            throw new IllegalArgumentException("Module " + newModule.getName() + " overlaps with "
+                    + getOverlappingEvent(newModule) + "!");
         }
     }
 
@@ -317,7 +318,8 @@ public void addCca(String ccaString) throws IllegalValueException {
         if (!isOverlapping(newCca)) {
             ccasList.add(newCca);
         } else {
-            throw new IllegalArgumentException("CCA " + newCca.getName() + " overlaps with " + getOverlappingEvent(newCca) + "!");
+            throw new IllegalArgumentException("CCA " + newCca.getName() + " overlaps with "
+                    + getOverlappingEvent(newCca) + "!");
         }
     }
 
@@ -374,10 +376,20 @@ public void addDatedEvent(String eventString) {
         if (!isOverlapping(newEvent)) {
             datedEventsList.add(newEvent);
         } else {
-            throw new IllegalArgumentException("Event " + newEvent.getName() + " overlaps with " + getOverlappingEvent(newEvent) + "!");
+            throw new IllegalArgumentException("Event " + newEvent.getName() + " overlaps with "
+                    + getOverlappingEvent(newEvent) + "!");
         }
     }
 
+    /**
+     * Adds a dated event to the schedule.
+     *
+     * @param event Dated event to be added.
+     */
+    public void addDatedEvent(DatedEvent event) {
+        datedEventsList.add(event);
+    }
+
     /**
      * Returns true if the given event overlaps with any event in the schedule
      * @param event
@@ -387,7 +399,7 @@ public boolean isOverlapping(TimeBlock event) {
         List<TimeBlock> totalList = new ArrayList<>();
         totalList.addAll(modulesList);
         totalList.addAll(ccasList);
-        totalList.addAll(datedEventsList); 
+        totalList.addAll(datedEventsList);
         for (TimeBlock e : totalList) {
             if (event.isOverlap(e)) {
                 return true;
@@ -405,7 +417,7 @@ public String getOverlappingEvent(TimeBlock event) {
         List<TimeBlock> totalList = new ArrayList<>();
         totalList.addAll(modulesList);
         totalList.addAll(ccasList);
-        totalList.addAll(datedEventsList); 
+        totalList.addAll(datedEventsList);
         for (TimeBlock e : totalList) {
             if (event.isOverlap(e)) {
                 return e.getName();
@@ -414,15 +426,6 @@ public String getOverlappingEvent(TimeBlock event) {
         return null;
     }
 
-    /**
-     * Adds a dated event to the schedule.
-     *
-     * @param event Dated event to be added.
-     */
-    public void addDatedEvent(DatedEvent event) {
-        datedEventsList.add(event);
-    }
-
     /**
      * Edits a dated event in the schedule.
      *

diff --git a/...c/commands/CommonFreetimeCommandTest.java → ...c/commands/CommonFreeTimeCommandTest.java b/...c/commands/CommonFreetimeCommandTest.java → ...c/commands/CommonFreeTimeCommandTest.java
@@ -21,7 +21,7 @@
  * Contains integration tests (interaction with the Model) for {@code CommonFreeTimeCommand}.
  */
 
-public class CommonFreetimeCommandTest {
+public class CommonFreeTimeCommandTest {
     private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs(), new UserData());
     private Model expectedModel = new ModelManager(getTypicalAddressBook(), new UserPrefs(), new UserData());
 
@@ -36,17 +36,17 @@ public void execute_userNoFreetime_failure() {
         model.setUser(JAMES);
         expectedModel.setUser(JAMES);
         CommonFreetimeCommand commonFreetimeCommand = new CommonFreetimeCommand();
-        assertCommandFailure(commonFreetimeCommand, model, CommonFreetimeCommand.MESSAGE_NO_CONTACTS);
+        assertCommandFailure(commonFreetimeCommand, model, CommonFreetimeCommand.MESSAGE_NO_FREE_TIME);
     }
 
     @Test
     public void execute_nameFriend_success() {
         CommonFreetimeCommand commonFreetimeCommand = new CommonFreetimeCommand(Index.fromOneBased(2));
         String expectedMessage = "You have common free times with Benson Meier at:" + "\n"
-                + "[Monday 0000 1730]" + "\n" + "[Monday 2000 2330]" + "\n"
-                + "[Tuesday 0000 2330]" + "\n" + "[Wednesday 0000 1130]" + "\n" + "[Wednesday 1300 2330]" + "\n"
-                + "[Thursday 0000 2330]" + "\n" + "[Friday 0000 2330]" + "\n" + "[Saturday 0000 2330]" + "\n"
-                + "[Sunday 0000 2330]" + "\n";
+                + "[Monday 0000 1800]" + "\n" + "[Monday 2000 2400]" + "\n"
+                + "[Tuesday 0000 2400]" + "\n" + "[Wednesday 0000 1200]" + "\n" + "[Wednesday 1300 2400]" + "\n"
+                + "[Thursday 0000 2400]" + "\n" + "[Friday 0000 2400]" + "\n" + "[Saturday 0000 2400]" + "\n"
+                + "[Sunday 0000 2400]" + "\n";
         assertCommandSuccess(commonFreetimeCommand, model, expectedMessage, expectedModel);
     }
 

diff --git a/...rser/CommonFreetimeCommandParserTest.java → ...rser/CommonFreeTimeCommandParserTest.java b/...rser/CommonFreetimeCommandParserTest.java → ...rser/CommonFreeTimeCommandParserTest.java
@@ -7,8 +7,8 @@
 import seedu.address.logic.commands.CommonFreetimeCommand;
 import seedu.address.logic.parser.exceptions.ParseException;
 
-public class CommonFreetimeCommandParserTest {
-    private CommonFreetimeCommandParser parser = new CommonFreetimeCommandParser();
+public class CommonFreeTimeCommandParserTest {
+    private CommonFreeTimeCommandParser parser = new CommonFreeTimeCommandParser();
 
     @Test
     public void parse_validArgs_returnsCommonFreetimeCommand() throws ParseException {

diff --git a/src/test/java/seedu/address/model/person/timetable/ModuleTest.java b/src/test/java/seedu/address/model/person/timetable/ModuleTest.java
@@ -55,7 +55,6 @@ public void editName_success() {
         assertThrows(IllegalArgumentException.class, () -> module.editName(""));
     }
 
-
     @Test
     public void isModule_success() {
         Module module = new Module("CS2103", "monday 1200 1400");
@@ -89,12 +88,14 @@ public void isValidModuleName() {
         assertFalse(Module.isValidModuleName("CS21*3")); // contains non-alphanumeric characters
         assertFalse(Module.isValidModuleName("CS2103 CS2103")); // contains spaces
         assertFalse(Module.isValidModuleName("CS21031")); // more numbers
-        assertFalse(Module.isValidModuleName("CS210")); // less numbers
         assertFalse(Module.isValidModuleName("CS")); // only alphbets
 
         // valid name
-        assertTrue(Cca.isValidCcaName("CS2103")); // alphanumeric characters
-        assertTrue(Cca.isValidCcaName("cs2103")); // with small letters
+        assertTrue(Module.isValidModuleName("CS210")); // 3 numbers
+        assertTrue(Module.isValidModuleName("CSS210")); // 3 starting alphabets
+        assertTrue(Module.isValidModuleName("CS2101S")); // less numbers
+        assertTrue(Module.isValidModuleName("CS2103")); // alphanumeric characters
+        assertTrue(Module.isValidModuleName("cs2103")); // with small letters
     }
 
     @Test

diff --git a/src/test/java/seedu/address/testutil/TypicalSchedule.java b/src/test/java/seedu/address/testutil/TypicalSchedule.java
@@ -17,13 +17,13 @@ public class TypicalSchedule {
             .build();
 
     public static final Schedule FULL_SCHEDULE = new ScheduleBuilder()
-            .withModule(new Module("CS2103", "Monday 0000 2330"))
-            .withModule(new Module("CS2101", "Tuesday 0000 2330"))
-            .withModule(new Module("CS2100", "Wednesday 0000 2330"))
-            .withModule(new Module("CS2103", "Thursday 0000 2330"))
-            .withModule(new Module("CS2101", "Friday 0000 2330"))
-            .withModule(new Module("CS2100", "Saturday 0000 2330"))
-            .withModule(new Module("CS2103", "Sunday 0000 2330"))
+            .withModule(new Module("CS2103", "Monday 0000 2400"))
+            .withModule(new Module("CS2101", "Tuesday 0000 2400"))
+            .withModule(new Module("CS2100", "Wednesday 0000 2400"))
+            .withModule(new Module("CS2103", "Thursday 0000 2400"))
+            .withModule(new Module("CS2101", "Friday 0000 2400"))
+            .withModule(new Module("CS2100", "Saturday 0000 2400"))
+            .withModule(new Module("CS2103", "Sunday 0000 2400"))
             .build();
 
     public static final Cca NORMAL_CCA = new Cca("Basketball", "Monday 1800 2000");