-
Notifications
You must be signed in to change notification settings - Fork 429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Chin Jung Hwan] iP #613
Open
jhwan0707
wants to merge
45
commits into
nus-cs2103-AY2425S2:master
Choose a base branch
from
jhwan0707:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
[Chin Jung Hwan] iP #613
Conversation
This file contains 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
In build.gradle, the dependencies on distZip and/or distTar causes the shadowJar task to generate a second JAR file for which the mainClass.set("seedu.duke.Duke") does not take effect. Hence, this additional JAR file cannot be run. For this product, there is no need to generate a second JAR file to begin with. Let's remove this dependency from the build.gradle to prevent the shadowJar task from generating the extra JAR file.
…changes. Load the data from the hard disk when the chatbot starts up.
DialogBox.fxml uses a padding of 15px on all sides, which creates excessive spacing around chat messages in the GUI. Reduced padding improves the compactness and readability of the dialog boxes, enhancing the visual appeal for users. Let's adjust padding in DialogBox.fxml to: - Reduce top and bottom from 15px to 10px for tighter vertical spacing - Reduce left and right from 15px to 8px for a balanced look These values maintain readability while aligning with typical UI design standards for chat interfaces. No additional CSS is needed as FXML adjustments suffice for this tweak.
The shadowJar task generates johan.jar with a fixed name, overwriting previous builds each time. Overwriting obscures build history, making it hard to track versions or revert to an earlier JAR if needed for debugging. Let's update shadowJar to: - Append a timestamp (e.g., yyyyMMdd-HHmmss) to the JAR filename - Produce unique files like johan-20250227-123456.jar This approach preserves build history without requiring a complex versioning scheme, suitable for quick iterations.
Johan class lacks explicit checks for critical assumptions, risking runtime errors if invariants are violated. Documenting assumptions with assertions improves code reliability and aids debugging by failing fast when conditions aren’t met. Let’s add assertions to: - Ensure loadedTasks is non-null after storage.loadTasks() - Validate non-null storage, tasks, and parser in GUI constructor - Check input is non-null and non-empty in executeCommand() Assertions are used instead of exceptions to signal developer errors, aligning with their intended use for internal checks.
Johan constructor mixes task loading with object initialization, violating single responsibility principle. Separating task loading improves code modularity and readability, making it easier to maintain and test independently. Let’s: - Extract task loading logic into a private loadTasks() method - Update console constructor to use loadTasks() for TaskList creation A separate method is preferred over inline logic to keep the constructor focused on initialization, aligning with code quality best practices.
executeCommand processes commands without leveraging Java Streams, relying on traditional loops in command implementations. Streams can simplify task processing (e.g., filtering, mapping), enhancing code conciseness and aligning with modern Java practices. Let’s add a TODO comment in executeCommand to: - Mark where Streams will be used for future task filtering - Plan integration with command execution flow A placeholder ensures we revisit this for optimization without disrupting current functionality.
A-Assertions
The master branch lacks the Streams placeholder added in branch-A-Streams, which marks future enhancements in the Parser. Incorporating Streams will enable modern Java practices like filtering tasks efficiently, improving code scalability. Let's merge branch-A-Streams to: - Introduce a TODO comment in Parser.parse() for Streams usage - Prepare for future task processing optimizations This merge integrates a planned enhancement without altering current functionality, ensuring a smooth transition to Streams later.
Johan lacks the ability to sort tasks, limiting organization options. Sorting tasks (e.g., deadlines chronologically) improves usability by presenting tasks in a logical order, enhancing user experience. Let's: - Make Task implement Comparable for default alphabetical sorting - Override compareTo in Deadline for chronological sorting by date - Add sort() method to TaskList using Collections.sort - Introduce SortCommand to trigger sorting via 'sort' command - Update Parser to handle 'sort' This approach uses Comparable for simplicity and flexibility, allowing custom sorting per task type.
Johan lacks a user guide, leaving users without clear instructions. A guide enhances usability by detailing all key features, meeting the A-UserGuide requirement for a product website. Let's: - Create docs/README.md with Johan Chatbot guide and Ui.png - Cover adding deadlines, managing, sorting, searching, and exiting - Prepare for GitHub Pages under /docs This uses GFMD with a template structure, keeping it concise and user-friendly.
Welcome message lacks guidance on initial actions for new users. Adding a hint improves onboarding by suggesting common commands. Let's: - Update showWelcome to include 'list' or 'sort' suggestion This keeps the message concise while aiding first-time users.
SortCommand message is wordy and slightly unclear to users. A concise, clear message improves readability and user understanding. Let's: - Change to 'Tasks sorted: deadlines by date, others by name' This aligns with Johan’s sorting logic in a user-friendly way.
User guide lacks a concrete example for the sort feature. An example clarifies how sorting works, enhancing user comprehension. Let's: - Add before/after example for 'sort' in Sorting Tasks section This keeps the guide practical and aligned with Johan’s features.
README lacks a comment to identify its purpose. A header comment clarifies the file’s intent for maintainers. Let's add a simple comment above the title.
Quick Start has redundant wording that’s less concise. Simplifying improves readability for users. Let's change 'Run it' to 'Run'.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Johan: Your Task-Taming Sidekick 🦸♂️
"The secret of getting ahead is getting started." – Mark Twain (source)
Johan is here to kick your task clutter to the curb! It’s:
Slow? Nope!Blazing fast to useHow to Get Started
Features
If you’re a Java ninja, tweak it to sharpen your skills! Check out the main method:
Johan’s FREE and ready to roll—your tasks won’t know what hit ‘em! 😉