-
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
[Ong Jing Chun] iP #447
base: master
Are you sure you want to change the base?
[Ong Jing Chun] iP #447
Conversation
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM. Just need to fix several parts of your code and you are all good! Do also ensure that you follow the checkstyle closely!
src/main/java/JacobMalon.java
Outdated
import java.util.Scanner; | ||
import taskscommand.*; | ||
|
||
public class JacobMalon { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on the implementation! 🎉 To improve readability and maintainability, I suggest adding Javadoc comments for both your classes and methods. This will help clarify their purpose, expected inputs, return values, and any potential exceptions they might throw. Providing detailed documentation will also make it easier for others (and your future self!) to understand and use your code effectively. Keep up the good work! 👍
src/main/java/JacobMalon.java
Outdated
break; | ||
} else if (command.equals("list")) { | ||
taskManager.listTasks(); | ||
} else if (command.startsWith("mark ")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting use of the startsWith() command! I didn't think of that!
src/main/java/JacobMalon.java
Outdated
} else if (command.startsWith("deadline ")) { | ||
String[] parts = command.substring(9).split(" /by ", 2); | ||
if (parts.length < 2) { | ||
throw new IllegalArgumentException("deadline command must be followed by description and date (e.g., deadline return book /by 2019-10-15)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good use of IllegalArgumentException()
! However, you might consider defining a custom task-related exception instead. This would make error handling more specific and prevent potential confusion in the future if other parts of the codebase also throw IllegalArgumentException()
for different reasons. A custom exception would improve clarity, maintainability, and debugging.
private void loadFromFile() { | ||
try { | ||
File directory = new File("data"); | ||
if (!directory.exists()) { | ||
directory.mkdir(); | ||
} | ||
|
||
File file = new File(FILE_PATH); | ||
if (!file.exists()) { | ||
file.createNewFile(); | ||
return; | ||
} | ||
|
||
BufferedReader reader = new BufferedReader(new FileReader(file)); | ||
String line; | ||
while ((line = reader.readLine()) != null) { | ||
// Parse the line and create appropriate task | ||
// This is a basic implementation - you might want to enhance it | ||
if (line.startsWith("[T]")) { | ||
tasks.add(new ToDo(line.substring(7))); | ||
} else if (line.startsWith("[D]")) { | ||
String[] parts = line.split(" \\(by: "); | ||
String description = parts[0].substring(7); | ||
String dateStr = parts[1].substring(0, parts[1].length() - 1); | ||
tasks.add(new Deadline(description, dateStr)); | ||
} else if (line.startsWith("[E]")) { | ||
String[] parts = line.split(" \\(from: | to: "); | ||
tasks.add(new Event(parts[0].substring(7), parts[1], parts[2].substring(0, parts[2].length() - 1))); | ||
} | ||
} | ||
reader.close(); | ||
} catch (IOException e) { | ||
System.out.println("Error loading from file: " + e.getMessage()); | ||
} | ||
} | ||
|
||
// Add this method to save tasks to file | ||
private void saveToFile() { | ||
try { | ||
FileWriter writer = new FileWriter(FILE_PATH); | ||
for (Task task : tasks) { | ||
writer.write(task.toString() + "\n"); | ||
} | ||
writer.close(); | ||
} catch (IOException e) { | ||
System.out.println("Error saving to file: " + e.getMessage()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might consider abstracting this chunk of code responsible for saving data into a separate class, such as SaveFile.java
. This would improve code readability, maintainability, and separation of concerns, making it easier to understand and debug in the future.
public void listTasksByDate(String date) { | ||
try { | ||
LocalDate queryDate = LocalDate.parse(date, INPUT_FORMATTER); | ||
List<Task> tasksOnDate = tasks.stream() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting use of stream()! I did not think of that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be beneficial to implement JUnit tests for this and other files to verify that each function runs correctly when given different inputs. This will help ensure the stability of your project, especially as you introduce more modifications in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- could be better titled as add date support
- can it support time or other date formats? e.g. 12.12.2025 1800
Improve code quality
Implement assertions
Task-related classes lack defensive programming measures to catch programming errors early in development. Let's add assertions to: * Validate task descriptions are non-null and non-empty * Ensure deadline dates are properly parsed * Guard against invalid task indices * Verify task list integrity after operations * Protect against null parameters in public methods These assertions help catch programming errors during development and make the code's assumptions explicit. They complement existing exception handling which deals with runtime errors. Part of code quality improvements for ip project.
Task management classes have several code quality issues including deep nesting, long methods, and unclear variable names. Let's improve the code quality by: * Extract helper methods to reduce method length and nesting * Add meaningful constants for magic strings * Group related operations together * Improve method and variable naming * Separate concerns between task and file operations * Add comprehensive JavaDoc comments * Apply SLAP (Single Level of Abstraction Principle) These changes make the code more maintainable and readable while preserving the existing functionality. Methods now follow a clear logical structure and operate at consistent abstraction levels. The improvements align with the coding quality guidelines: - Methods are kept under 30 LOC - Nesting is limited to 2-3 levels - Each method has a single responsibility - Code is self-documenting where possible Part of ongoing code quality improvements for ip project.
Tasks can currently be added multiple times without any warning, which may lead to confusion and cluttered task lists. Let's add duplicate detection that: * Identifies duplicate tasks based on their content * Warns users when they try to add a duplicate task * Handles different task types appropriately: - Todo: checks description equality - Deadline: checks description and deadline timing - Event: checks description and event timing The implementation allows duplicates to be added after warning the user, providing flexibility while preventing accidental duplicates. Part of C-DetectDuplicates extension implementation.
- Add tests for file I/O operations in TaskManager - Test file creation when storage file doesn't exist - Verify correct date/time formatting for tasks - Test save/load functionality with different task types - Add test cases for input format consistency - Ensure proper handling of empty storage file What's changed: - Added new test cases for file operations - Updated existing tests to match new date format - Added edge case tests for file handling - Improved test coverage for task formatting
- Implement consistent date/time format validation - Use shared INPUT_FORMATTER (d/M/yyyy HHmm) across all classes - Add proper date parsing and validation in JacobMalon - Improve error messages with format examples - Add validation for event start/end time order - Update file I/O to maintain date format consistency What's changed: - JacobMalon now validates dates before creating tasks - Consistent date format validation across Deadline and Event - Better error messages with examples - Proper handling of invalid date formats
Date/time formats are inconsistent across different parts of the app. This causes confusion for users and makes the code harder to maintain. Let's standardize the date/time handling by: * Using shared INPUT_FORMATTER (d/M/yyyy HHmm) across all classes * Adding proper validation before task creation * Improving error messages with format examples * Validating event start/end time order This standardization: * Makes the user experience more consistent * Reduces code duplication * Improves error handling * Makes the codebase more maintainable The validation is done at input level rather than entity level to fail fast and provide better user feedback.
Tasks are lost when the application is closed. Users need their tasks to persist between sessions to make the app useful. Let's implement file-based storage that: * Creates list.txt if it doesn't exist * Saves tasks in user input format * Loads tasks on application startup * Handles different task types (Todo/Deadline/Event) This implementation: * Preserves exact user input format for easier editing * Maintains task status across sessions * Provides graceful handling of missing storage file * Ensures backward compatibility The storage format matches user input commands to maintain consistency between what users type and what they see in the file.
Error messages lack specific examples, making it difficult for users to understand the correct input format. This leads to user frustration and increased support queries. Let's improve error messages by: * Adding format examples (e.g., 2/12/2023 1800) * Clarifying command syntax with real-world examples * Standardizing message format across all commands * Including complete command examples Examples now show: * Todo: todo read book * Deadline: deadline return book /by 2/12/2023 1800 * Event: event team meeting /from 2/12/2023 1400 /to 2/12/2023 1600 Better error messages reduce user confusion and make the application more user-friendly for first-time users.
Test coverage for date/time validation is incomplete. Invalid formats could slip through, causing runtime errors and poor user experience. Let's add comprehensive test cases for: * Valid date formats (d/M/yyyy HHmm) * Invalid date patterns * Edge cases (single digit dates/months) * Event time order validation * Boundary conditions (year transitions) These tests ensure: * Consistent date/time handling * Proper error messages for invalid inputs * Robust validation across all task types * Prevention of invalid event time ranges Added both unit and integration tests to verify the entire validation pipeline from user input to task creation.
Date formatters are duplicated across Task classes, violating DRY principle. Changes to date format require updates in multiple places. Let's create a DateTimeUtil class that: * Centralizes all date/time formatting logic * Provides shared INPUT_FORMATTER and OUTPUT_FORMATTER * Adds validation methods for date strings * Handles parsing exceptions consistently Benefits: * Single source of truth for date formats * Easier maintenance and format changes * Consistent error handling * Reduced code duplication The utility class is package-private to maintain encapsulation while allowing reuse within the tasks package.
Image naming doesn't follow project conventions where only the first letter should be capitalized. This inconsistency affects documentation and README rendering. Let's: * Rename UI.png to Ui.png * Update all references to the image * Maintain consistent naming convention * Ensure proper image loading in documentation This change aligns with: * Project naming conventions * Java class naming standards * Documentation best practices The new filename matches other UI-related components in the project, improving overall code consistency.
JacobMalon - Your Personal Task Manager 📋
A Java-based task management application that helps you stay organized. Built with JavaFX for a modern user interface and robust task tracking capabilities.
Key Features 🚀
list.txt
Getting Started
java -jar jacobmalon.jar
todo read book
- Create a tododeadline return book /by 2/12/2023 1800
- Set a deadlineevent team meeting /from 2/12/2023 1400 /to 2/12/2023 1600
- Schedule an eventExample Usage
Project Status
Technologies Used
Command Line Interface(replaced with GUI)For more information about the project, visit our GitHub Repository.
Acknowledgments
Special thanks to: