From e77f11a97683975fd9dec347541c6b1259a20a0a Mon Sep 17 00:00:00 2001 From: trigg770 Date: Thu, 10 Sep 2026 19:32:32 +0800 Subject: [PATCH 1/2] Encapsulate task fields and fix stale comments Task, Deadline and Event keep their fields protected and mutable, and TaskList reads a task's description straight from the field. Deadline and Event each hold their own copy of the display date format. Several comments no longer match the code. ExitCommand describes the old conversation loop, Ui's show methods say they print, and TaskList#asList says it returns a view when it returns a copy. DialogBox catches a failed FXML load to keep the app running, yet the next line fails with a NullPointerException anyway. Fields that any class in the package can reassign undercut the guarantees Task exists to give, and comments that contradict the code mislead the next reader. Let's * make the task fields private, and final where they never change, with Task#getDescription() for the search to use * move the display format into Task so both subclasses share one copy * correct the stale comments and Javadoc * rethrow a failed DialogBox load as an IllegalStateException, since the FXML ships in the JAR and a missing one means a broken build Co-Authored-By: Claude Opus 5 --- src/main/java/ted/DialogBox.java | 5 +++-- src/main/java/ted/Ui.java | 6 +++--- src/main/java/ted/command/ExitCommand.java | 4 ++-- src/main/java/ted/task/Deadline.java | 7 +------ src/main/java/ted/task/Event.java | 9 ++------- src/main/java/ted/task/Task.java | 19 +++++++++++++++++-- src/main/java/ted/task/TaskList.java | 5 +++-- 7 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/main/java/ted/DialogBox.java b/src/main/java/ted/DialogBox.java index 6766f3b0a6..ea1bf0bc6f 100644 --- a/src/main/java/ted/DialogBox.java +++ b/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); diff --git a/src/main/java/ted/Ui.java b/src/main/java/ted/Ui.java index e22a3d8747..e2551bfe82 100644 --- a/src/main/java/ted/Ui.java +++ b/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. diff --git a/src/main/java/ted/command/ExitCommand.java b/src/main/java/ted/command/ExitCommand.java index e6f9657815..76ba60c33c 100644 --- a/src/main/java/ted/command/ExitCommand.java +++ b/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. } /** diff --git a/src/main/java/ted/task/Deadline.java b/src/main/java/ted/task/Deadline.java index 44f48e3b35..35da2615b4 100644 --- a/src/main/java/ted/task/Deadline.java +++ b/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. diff --git a/src/main/java/ted/task/Event.java b/src/main/java/ted/task/Event.java index b5ec8ea5cd..81915a1bd5 100644 --- a/src/main/java/ted/task/Event.java +++ b/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. diff --git a/src/main/java/ted/task/Task.java b/src/main/java/ted/task/Task.java index 67dc2d9c12..ef39529e2e 100644 --- a/src/main/java/ted/task/Task.java +++ b/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. * diff --git a/src/main/java/ted/task/TaskList.java b/src/main/java/ted/task/TaskList.java index d9f3a17753..4c9a3662b9 100644 --- a/src/main/java/ted/task/TaskList.java +++ b/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 asList() { return List.copyOf(tasks); From 727f04f48ab5eb0645249d8f392e5abc94e0ecaf Mon Sep 17 00:00:00 2001 From: trigg770 Date: Thu, 10 Sep 2026 19:33:18 +0800 Subject: [PATCH 2/2] Fix option parsing and untrimmed GUI input Parser locates /by, /from and /to with indexOf, which also matches them inside a word. "event lunch w/tom /from ... /to ..." is rejected because the /to in "w/tom" is taken for the separator. The GUI passes its text field to Ted#getResponse as typed, and only the terminal trims input, so " list" in the GUI is an unknown command. A warning about the save file also appears above the welcome banner, because the constructor queues it before the greeting exists. The first two reject ordinary input, and all three show up in the GUI, which is the version people use. Let's * accept an option only as a standalone word * trim input in Ted#getResponse, where the terminal and the GUI meet, rather than in each front end * hold the startup warnings back until getGreeting has shown the welcome * add a regression test for each case Co-Authored-By: Claude Opus 5 --- src/main/java/ted/Parser.java | 24 +++++++++++++++++--- src/main/java/ted/Ted.java | 28 ++++++++++++++++------- src/test/java/ted/ParserTest.java | 17 ++++++++++++++ src/test/java/ted/TedTest.java | 37 +++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 11 deletions(-) create mode 100644 src/test/java/ted/TedTest.java diff --git a/src/main/java/ted/Parser.java b/src/main/java/ted/Parser.java index 6e2ed48bde..5740763887 100644 --- a/src/main/java/ted/Parser.java +++ b/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("(? 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(); } diff --git a/src/test/java/ted/ParserTest.java b/src/test/java/ted/ParserTest.java index c989582078..767a957703 100644 --- a/src/test/java/ted/ParserTest.java +++ b/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")); diff --git a/src/test/java/ted/TedTest.java b/src/test/java/ted/TedTest.java new file mode 100644 index 0000000000..91b6cb0473 --- /dev/null +++ b/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. + *

+ * 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); + } +}