[ananya21] iP #464
Open
[ananya21] iP #464
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
wilsonwid
reviewed
Feb 6, 2024
There was a problem hiding this comment.
The code in this PR is of an overall good standard, even though there are several places where it could be improved. I commented on several coding standard violations, most of which are due to missing Javadoc comments. It would also be good to review the imports used in each of the classes to see whether they are unused.
src/main/java/Charlie.java
Outdated
| @@ -0,0 +1,59 @@ | |||
| import java.util.ArrayList; | |||
| import java.util.Scanner; | |||
|
|
|||
There was a problem hiding this comment.
It would be good to add a Javadoc comment for the class, as well as the public methods inside the class.
src/main/java/Charlie.java
Outdated
| @@ -0,0 +1,59 @@ | |||
| import java.util.ArrayList; | |||
There was a problem hiding this comment.
It would be good to place all files in a package.
src/main/java/Task.java
Outdated
| protected String description; | ||
| protected boolean isDone; | ||
|
|
||
| public Task(String description) { |
There was a problem hiding this comment.
It would be good to add Javadoc comments to public methods and to the class.
src/main/java/Task.java
Outdated
| return (isDone ? "[X]" : "[ ]"); // mark done task with X | ||
| } | ||
|
|
||
| public void markAsDone() { |
There was a problem hiding this comment.
Could this method and markAsNotDone() perhaps be fused into one method with a boolean parameter?
src/main/java/Todo.java
Outdated
| } | ||
| @Override | ||
| public String toString() { | ||
| return "[T]" + super.toString(); |
There was a problem hiding this comment.
I like how inheritance and polymorphism are used to shorten this line.
NusMinato
reviewed
Feb 6, 2024
Ensure Task objects are in a consistent state In our Task class, certain assumptions about the state of Task objects hold true throughout the lifecycle of these objects (e.g., description cannot be null). Previously, these assumptions were not explicitly checked, leading to a risk of encountering NullPointerExceptions if these assumptions ever got violated due to future changes. Let's, * add assertions to the constructor to ensure the description is not null * add assertions in setStatus method to validate the state transitions Adding these assertions helps catch bugs during the development phase and ensures our assumptions about the Task object's state are always validated.
Extract command initialization into dedicated methods The Parser class's single-method approach for command parsing was becoming cumbersome and difficult to maintain. This refactoring aims to improve the code's readability and maintainability by adhering to the Single Responsibility Principle. Let's, * Create separate methods for each command's parsing logic, such as , , etc. * Ensure each method is responsible for a single aspect of the command parsing process. This change not only makes the Parser class easier to understand and modify but also simplifies adding new commands in the future. By breaking down the complex method into smaller, focused methods, we improve the code quality significantly, making it easier for future developers to navigate and extend the application.
Introduce the ability to assign priority levels to tasks, allowing users to mark tasks with a specific priority. This enhancement enables more effective task management by emphasizing the urgency or importance of certain tasks over others. Modifications include: - Update the Parser class to recognize priority commands and extract the relevant priority level from user input. - Adjust the Duke class to handle the assignment of priority levels to tasks, storing this information accordingly. This change addresses the need for users to organize their tasks by priority, facilitating a more efficient and targeted approach to task management. Priority levels are defined as high, medium, and low, corresponding to numeric values for easy comparison. The implementation supports dynamic adjustment of priorities, allowing users to update the priority level of tasks as needed.
Enhance the User Guide in the README with detailed descriptions of new features This update enriches the README file with a comprehensive guide to the application's features, improving user understanding and accessibility. The changes include: - Detailed instructions on how to add todo, deadline, and event tasks, including how users are prompted for task prioritization. - Explanation of the find keyword feature for locating tasks related to specific keywords. - Guidelines on deleting tasks, listing all tasks, marking tasks as completed, and unmarking tasks. - Usage examples for each command, providing clear scenarios that demonstrate the expected outcomes. These enhancements aim to provide users with clear and concise instructions on utilizing the application's functionality to its full extent, promoting a more intuitive and user-friendly experience. Additionally, the inclusion of expected outcomes for each command usage example serves to set clear expectations, aiding users in verifying the correct application behavior.
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.
Project Charlie Enhancement 🚀
This PR introduces several enhancements to the Project Charlie, improving its performance and user experience. Here's what's new:
Features:
How to Get Started:
What's Included in This PR: