A-CodeQuality: Improve code quality - #1
Merged
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Increment: A-CodeQuality
Task#getDescription(), and moves the shared date format intoTask.DialogBoxFXML load as anIllegalStateException./by,/fromand/toonly as standalone words, so "lunch w/tom" is no longer split at "/to".Ted#getResponse, so GUI input with leading spaces works, and shows save-file warnings after the welcome.🤖 Generated with Claude Code