-
Notifications
You must be signed in to change notification settings - Fork 484
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
[ChangruHenryQian] iP #544
base: master
Are you sure you want to change the base?
[ChangruHenryQian] iP #544
Conversation
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 well done! Just some minor changes to be made. Can consider abstracting certain methods to different classes.
src/main/java/Duke.java
Outdated
private Storage storage; | ||
private TaskList tasks; | ||
private Ui ui = new Ui(); | ||
private static final String filePath = "./data/duke.txt"; |
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.
Since it is a constant, may I suggest this change?
private static final String filePath = "./data/duke.txt"; | |
private static final String FILE_PATH = "./data/duke.txt"; |
src/main/java/duke/Parser.java
Outdated
} | ||
|
||
public void parse() { | ||
try{ |
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.
using switch statements may help reduce nesting. what do you think?
src/main/java/duke/Parser.java
Outdated
this.taskList.addTask(newToDo); | ||
} else if (command.startsWith("deadline")) { | ||
LocalDate deadline = LocalDate.parse(command.split(" /by ", 2)[1]); | ||
String name = command.split(" /by ", 2)[0].split(" ", 2)[1]; |
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.
Perhaps splitting them into multiple statements could improve the readability of the code. It can make it easier for you to debug your code too :)
@@ -0,0 +1,83 @@ | |||
package duke; | |||
|
|||
import java.io.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.
I like how you have organised your imports according to their functionality. It's clear and easy to read. Great job!
return "[" + this.getStatusIcon() + "] " + this.description; | ||
} | ||
|
||
public String toTxt() { |
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.
just a trivial issue but would spelling out the txt improve the readability of your code?
After implementing GUI, the entry point was changed. Let's change mainClass into duke.Launcher.
The functional code is not capable of checking assumptions. Assumptions may not be true in some cases and checking is required. Let's add assert feature to document important assumptions.
Some parts of the code have bad code quality. Improvements on code quality increase readability. Let's improve code quality through methods such as refactoring.
Improve code quality
# Conflicts: # src/main/java/duke/Storage.java
Implement assertions
Jarvis Chat Bot
Jarvis frees your mind of having to remember things you need to do. It's,
FASTSUPER FAST to useAll you need to do is,
And it is FREE!
Features:
If you Java programmer, you can use it to practice Java too. Here's the
main
method: