A-CodeQuality: Improve code quality

src/main/java/ted/DialogBox.java

@@ -34,8 +34,9 @@ private DialogBox(String text, Image image) {
             fxmlLoader.setRoot(this);
             fxmlLoader.load();
         } catch (IOException e) {
-            // A message that cannot be drawn is not worth stopping the app for.
-            e.printStackTrace();
+            // The FXML ships inside the JAR, so failing to load it means a broken
+            // build. Carrying on would only fail later with a NullPointerException.
+            throw new IllegalStateException("Cannot load DialogBox.fxml", e);
         }
 
         dialog.setText(text);

src/main/java/ted/Parser.java

@@ -4,6 +4,8 @@
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeParseException;
 import java.time.format.ResolverStyle;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import ted.command.AddCommand;
 import ted.command.Command;
@@ -112,7 +114,7 @@ private static Deadline parseDeadline(String argument) throws TedException {
         String example = "for example: deadline return book /by 2/12/2019 1800";
         requireNotBlank(argument, "A deadline needs a description and a due time, " + example);
 
-        int separator = argument.indexOf(OPTION_BY);
+        int separator = findOption(argument, OPTION_BY);
         if (separator == -1) {
             throw new TedException("I need to know when this is due. Use /by, " + example);
         }
@@ -137,8 +139,8 @@ private static Event parseEvent(String argument) throws TedException {
         String example = "for example: event project meeting /from 2/12/2019 1400 /to 2/12/2019 1600";
         requireNotBlank(argument, "An event needs a description, a start and an end, " + example);
 
-        int fromSeparator = argument.indexOf(OPTION_FROM);
-        int toSeparator = argument.indexOf(OPTION_TO);
+        int fromSeparator = findOption(argument, OPTION_FROM);
+        int toSeparator = findOption(argument, OPTION_TO);
         if (fromSeparator == -1 || toSeparator == -1) {
             throw new TedException("An event needs both /from and /to, " + example);
         }
@@ -205,6 +207,22 @@ private static LocalDateTime parseDateTime(String text, String example) throws T
         }
     }
 
+    /**
+     * Returns where an option such as {@code /to} starts in the argument.
+     * Only a standalone word counts, so that the {@code /to} inside a
+     * description like "lunch w/tom" is not mistaken for the separator.
+     *
+     * @param argument everything the user typed after the command word.
+     * @param option   the option to look for, e.g. {@code /by}.
+     * @return index of the option's first standalone use, or -1 if there is none.
+     */
+    private static int findOption(String argument, String option) {
+        // The lookarounds require whitespace or the edge of the text on both
+        // sides, without making that whitespace part of the match.
+        Matcher matcher = Pattern.compile("(?<!\\S)" + Pattern.quote(option) + "(?!\\S)").matcher(argument);
+        return matcher.find() ? matcher.start() : -1;
+    }
+
     /**
      * Rejects input the user left out.
      *

src/main/java/ted/Ted.java

@@ -30,6 +30,13 @@ public class Ted {
     /** The tasks entered so far. */
     private TaskList tasks;
 
+    /**
+     * Why the save file could not be read, or {@code null} if it loaded.
+     * Kept until the greeting so that the warning follows the welcome banner
+     * instead of appearing above it.
+     */
+    private String loadErrorMessage;
+
     /** Whether the last handled command asked Ted to stop. */
     private boolean isExit = false;
 
@@ -55,20 +62,16 @@ public Ted(String filePath) {
             // The list must be complete before the user is greeted, so that every
             // command that follows can rely on it.
             tasks = new TaskList(storage.load());
-            if (storage.getSkippedLineCount() > 0) {
-                ui.showSkippedLines(storage.getSkippedLineCount());
-            }
         } catch (TedException e) {
             // An unreadable save file is not worth refusing to start over.
-            ui.showLoadingError(e.getMessage());
+            loadErrorMessage = e.getMessage();
             tasks = new TaskList();
         }
     }
 
     /** Greets the user, then handles commands until the conversation ends. */
     public void run() {
-        ui.showWelcome();
-        ui.printReply(ui.flush());
+        ui.printReply(getGreeting());
 
         // hasNextCommand() also stops the loop when the input stream ends,
         // e.g. on Ctrl-D or at the end of a piped file.
@@ -95,14 +98,17 @@ public void run() {
      * @return Ted's reply, or an empty string if there was nothing to reply to.
      */
     public String getResponse(String input) {
-        if (input.isBlank()) {
+        // The GUI passes its text field on as typed, so stray spaces are removed
+        // here, where both front ends meet, rather than in each of them.
+        String trimmedInput = input.strip();
+        if (trimmedInput.isEmpty()) {
             // A stray blank line is not worth a reply.
             return "";
         }
 
         Command command;
         try {
-            command = Parser.parse(input);
+            command = Parser.parse(trimmedInput);
         } catch (TedException e) {
             // Every problem Ted can recognise is recoverable, so the message is
             // shown and the conversation continues with the next command.
@@ -132,6 +138,12 @@ public String getResponse(String input) {
      */
     public String getGreeting() {
         ui.showWelcome();
+        if (loadErrorMessage != null) {
+            ui.showLoadingError(loadErrorMessage);
+        }
+        if (storage.getSkippedLineCount() > 0) {
+            ui.showSkippedLines(storage.getSkippedLineCount());
+        }
         return ui.flush();
     }
 

src/main/java/ted/Ui.java

@@ -164,7 +164,7 @@ public void showMarked(Task task, boolean isDone) {
     }
 
     /**
-     * Prints every stored task as a numbered list, starting from 1.
+     * Shows every stored task as a numbered list, starting from 1.
      *
      * @param tasks the tasks to show.
      */
@@ -173,7 +173,7 @@ public void showTasks(TaskList tasks) {
     }
 
     /**
-     * Prints the tasks that matched a search, as a numbered list.
+     * Shows the tasks that matched a search, as a numbered list.
      * The numbers count the matches, not the positions in the full list, so
      * they are not the numbers to pass to mark or delete.
      *
@@ -186,7 +186,7 @@ public void showMatchingTasks(TaskList matches, String keyword) {
     }
 
     /**
-     * Prints tasks as a numbered list, starting from 1.
+     * Adds tasks to the reply as a numbered list, starting from 1.
      *
      * @param tasks        the tasks to show.
      * @param header       line introducing the list.

src/main/java/ted/command/ExitCommand.java

@@ -13,8 +13,8 @@ public ExitCommand() {
     /** Does nothing: leaving needs no work beyond stopping the loop. */
     @Override
     public void execute(TaskList tasks, Ui ui, Storage storage) {
-        // Nothing to do: the goodbye is shown by Ted once the loop has stopped,
-        // so that it is not framed like an ordinary reply.
+        // Nothing to do: Ted shows the goodbye itself when it sees isExit(),
+        // so that the goodbye replaces an ordinary reply.
     }
 
     /**

src/main/java/ted/task/Deadline.java

@@ -1,19 +1,14 @@
 package ted.task;
 
 import java.time.LocalDateTime;
-import java.time.format.DateTimeFormatter;
 
 /**
  * A task that must be done before a given point in time,
  * e.g. {@code submit report (by: 2 Dec 2019, 6:00 PM)}.
  */
 public class Deadline extends Task {
-    /** Format used to present the due date and time to the user. */
-    private static final DateTimeFormatter DISPLAY_DATE_TIME_FORMAT =
-            DateTimeFormatter.ofPattern("d MMM uuuu, h:mm a");
-
     /** When the task is due, represented as a date and time. */
-    protected LocalDateTime by;
+    private final LocalDateTime by;
 
     /**
      * Creates a deadline that is not done yet.

src/main/java/ted/task/Event.java

@@ -1,22 +1,17 @@
 package ted.task;
 
 import java.time.LocalDateTime;
-import java.time.format.DateTimeFormatter;
 
 /**
  * A task that runs from one point in time to another,
  * e.g. {@code project meeting (from: 2 Dec 2019, 2:00 PM to: 2 Dec 2019, 4:00 PM)}.
  */
 public class Event extends Task {
-    /** Format used to present the start and end date-times to the user. */
-    private static final DateTimeFormatter DISPLAY_DATE_TIME_FORMAT =
-            DateTimeFormatter.ofPattern("d MMM uuuu, h:mm a");
-
     /** When the event starts. */
-    protected LocalDateTime from;
+    private final LocalDateTime from;
 
     /** When the event ends. */
-    protected LocalDateTime to;
+    private final LocalDateTime to;
 
     /**
      * Creates an event that is not done yet.

src/main/java/ted/task/Task.java

@@ -1,5 +1,7 @@
 package ted.task;
 
+import java.time.format.DateTimeFormatter;
+
 /**
  * A single task that Ted keeps track of.
  * Bundles a task's description with its done status, so that the two can no
@@ -14,11 +16,15 @@ public abstract class Task {
     /** Separator between fields in the save file. */
     public static final String SAVE_FIELD_SEPARATOR = " | ";
 
+    /** Format used to present dates and times to the user, shared by every kind of task. */
+    protected static final DateTimeFormatter DISPLAY_DATE_TIME_FORMAT =
+            DateTimeFormatter.ofPattern("d MMM uuuu, h:mm a");
+
     /** What the user wants to get done. */
-    protected String description;
+    private final String description;
 
     /** Whether the task has been completed. */
-    protected boolean isDone;
+    private boolean isDone;
 
     /**
      * Creates a task that is not done yet.
@@ -30,6 +36,15 @@ public Task(String description) {
         this.isDone = false;
     }
 
+    /**
+     * Returns what the user wants to get done.
+     *
+     * @return the task's description.
+     */
+    public String getDescription() {
+        return description;
+    }
+
     /**
      * Returns the icon shown in place of a tick box.
      *

src/main/java/ted/task/TaskList.java

@@ -95,7 +95,7 @@ public TaskList find(String keyword) {
         String lowerCaseKeyword = keyword.toLowerCase();
         TaskList matches = new TaskList();
         for (Task task : tasks) {
-            if (task.description.toLowerCase().contains(lowerCaseKeyword)) {
+            if (task.getDescription().toLowerCase().contains(lowerCaseKeyword)) {
                 matches.add(task);
             }
         }
@@ -105,7 +105,8 @@ public TaskList find(String keyword) {
     /**
      * Returns the tasks as a plain list, for code that only needs to read them.
      *
-     * @return an unmodifiable view of the tasks, in order.
+     * @return an unmodifiable copy of the tasks, in order, which later
+     *         changes to this list do not affect.
      */
     public List<Task> asList() {
         return List.copyOf(tasks);

src/test/java/ted/ParserTest.java

@@ -154,6 +154,23 @@ public void parse_eventWithSeparatorsSwapped_exceptionThrown() {
         assertThrows(TedException.class, () -> Parser.parse("event meeting /to 2/12/2019 1600 /from 2/12/2019 1400"));
     }
 
+    @Test
+    public void parse_eventDescriptionContainingSlashTo_descriptionKept() throws TedException {
+        // "w/tom" contains "/to", which once was mistaken for the end-time separator.
+        TaskList tasks = new TaskList();
+        Parser.parse("event lunch w/tom /from 1/1/2026 1200 /to 1/1/2026 1300")
+                .execute(tasks, new SilentUi(), new NoOpStorage());
+        assertEquals("lunch w/tom", tasks.get(0).getDescription());
+    }
+
+    @Test
+    public void parse_deadlineDescriptionContainingSlashBy_descriptionKept() throws TedException {
+        TaskList tasks = new TaskList();
+        Parser.parse("deadline read w/bytes /by 1/1/2026 1200")
+                .execute(tasks, new SilentUi(), new NoOpStorage());
+        assertEquals("read w/bytes", tasks.get(0).getDescription());
+    }
+
     @Test
     public void parse_eventEndingBeforeStart_exceptionThrown() {
         assertThrows(TedException.class, () -> Parser.parse("event meeting /from 2/12/2019 1600 /to 2/12/2019 1400"));

src/test/java/ted/TedTest.java

@@ -0,0 +1,37 @@
+package ted;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Tests {@link Ted}'s one-message-at-a-time interface, which the GUI relies on.
+ * <p>
+ * Every test keeps its save file inside a {@link TempDir}, so running the tests
+ * can never disturb the user's own tasks.
+ */
+public class TedTest {
+    @Test
+    public void getResponse_leadingSpaces_commandRecognised(@TempDir Path tempDir) {
+        // The GUI passes its text field on untrimmed, so Ted must cope with padding.
+        Ted ted = new Ted(tempDir.resolve("ted.txt").toString());
+        assertTrue(ted.getResponse("   list").contains("You have no tasks yet."));
+    }
+
+    @Test
+    public void getGreeting_unreadableSaveLine_warningFollowsWelcome(@TempDir Path tempDir) throws IOException {
+        Path dataFile = tempDir.resolve("ted.txt");
+        Files.write(dataFile, List.of("not a task"));
+
+        String greeting = new Ted(dataFile.toString()).getGreeting();
+        int welcomeIndex = greeting.indexOf("Hello! I'm Ted.");
+        int warningIndex = greeting.indexOf("Skipped 1 unreadable line");
+        assertTrue(welcomeIndex >= 0 && warningIndex > welcomeIndex);
+    }
+}