-
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
[Caleb Cherng Kai Zhe] iP #468
base: master
Are you sure you want to change the base?
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.
Created initialization for Task class Added toString() method for Task class Added ability to set tasks as done
Updated Task.java to be able to mark tasks as done Added markDone() and markUndone() to Processor.java
Added EventTask.java Fixed some formatting errors
Added processing for DeadlineTask, TodoTask and EventTask
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.
Other than a few nits, LGTM!
src/main/java/DeadlineTask.java
Outdated
|
||
@Override | ||
public String toString() { | ||
return "[D]" + super.toString() + " (by: " + by + ")"; |
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.
I like the use of newlines to keep char count below 120
src/main/java/Processor.java
Outdated
import java.util.regex.Pattern; | ||
|
||
public class Processor { | ||
private List<Task> store = new ArrayList<>(); |
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.
private List<Task> store = new ArrayList<>(); | |
private List<Task> store = new ArrayList<>(); |
Should this be named more specific?
src/main/java/Angela.java
Outdated
import java.util.Scanner; | ||
import java.util.List; |
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.
import java.util.Scanner; | |
import java.util.List; | |
import java.util.List; | |
import java.util.Scanner; |
Perhaps your imports could be ordered lexicographically?
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, your code is easy to read and understand. However, there are still spaces to improve.
Good job!
src/main/java/Processor.java
Outdated
String input; | ||
do { | ||
input = textIn.nextLine(); | ||
if (input.equalsIgnoreCase("list")) { |
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.
Maybe you can use switch-case style to improve readability and maintainability in future
src/main/java/Processor.java
Outdated
output(out.trim(), true); | ||
} | ||
|
||
public void addTask(String input) { |
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.
maybe addTask(Task task) be better instead of 3 versions of addTask() for Event, Deadline and Todo, to avoid misusing
src/main/java/Processor.java
Outdated
input = textIn.nextLine(); | ||
if (input.equalsIgnoreCase("list")) { | ||
printList(); | ||
} else if (input.length() >= 4 && input.substring(0, 4).equalsIgnoreCase("mark")) { //probably move this 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.
you may use split(" ") to make an array String[] for convenience
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.
LGTM!
There a are a few minor refactorings that are non essential.
src/main/java/Angela.java
Outdated
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.
The order of imports could be lexicographical?
src/main/java/Processor.java
Outdated
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.
A switch case would be easier to follow in term of logic inside the start() function.
We can use .split() to get the different parts of the input?
Then once we get the command we continue to process based off of expected format.
Since the list of tasks is of type Task we can modify addTask to have signature addTask(Task t) as then we can add all 3 types without having to write 3 different methods.
src/main/java/Processor.java
Outdated
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.
We can use the java string package to more efficiently process input?
The .trim() and .split() method could remove the need for the trimInput functions
src/main/java/Task.java
Outdated
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.
We can make this class abstract, as we wont be instantiating any objects of type Task and we can prevent misuse of this class and enforce usage of the actual 3 types we expect.
Output messages can be concatenated into a class and called using its static methods to prevent code duplication
Cleaned up command processing by adding new function to split command input Used switch-case instead of if-else to polish code
More abstraction in the Processor class
Add testcases to input.txt and EXPECTED.TXT Modify runtest.bat
Add several Exceptions to handle cases of invalid input
Add ability to delete tasks that are made
Add getArguments method to ensure correct argument flags are passed in
Storage.java manages the storage of the tasks into local files. Edit Processor.java to integrate changes from Storage.java
Branch level 7
Branch level 8
Add Command.java class to act as data structure for commands
Add Parser.java and TaskManager.java to handle command parsing and editing the task list
Branch a more oop
Add method to find tasks by keyword
Fix coding standard issues
Add JavaDoc
Branch level 10
Add lastFunction local variable to Angela.java Let's create a variable that stores the last inputted command that was called by the user for use in the undo function. This allows for the undo method to recall the last function used and act accordingly in the future. Add new undo() method to Angela.java The method will recall the last function, calling an undo method in the manager if the last function made changes to the database. In the event of a bad undo (undoing a list, find command), the method will give an error message.
Task class modifications Task class and its subclasses are not effectively final. This caused problems when archiving the previous iteration of the task list. Let's change do/undo task methods to return a new copy of the task, allowing for easier archiving for the tasks. Add support for archiving task list in TaskManager Lets let TaskManager save a copy of the previous iteration of tasks before any changes to the tasklist such that it can be easily retrieved to undo the change.
Add assertions to Angela.java and Storage.java
Improve code quality
Add support for undoing commands
Event Tasks were saved in the wrong format
Angela
Based on Project Moon's Lobotomy Corporation, Angela is an advanced
humanAI whose role is to guide you through your tasks just as she did in LC.With the chat bot, you can enjoy
Features:
Todo (with priority!)
If you want to contribute, open a pull request to help improve my code! (My poor
processDb
function needs help 😭)