-
Notifications
You must be signed in to change notification settings - Fork 364
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
[wongyx] iP #112
base: master
Are you sure you want to change the base?
[wongyx] iP #112
Conversation
This file contains backend data for the program Files containing backend data should not be on the repo, hence it is deleted
Most of the code is written under main. Extracting closely related code into classes and placing them into packages improves the code quality. Let's create multiple classes to deal with related parts of the code and package them together.
Mark, Unmark and Delete Commands do not save after execution. This commits fixes these issues.
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 change code to follow the coding standard, but overall I like it.
src/main/java/duke/Parser.java
Outdated
throws TaskNoDescriptionException, NotEnoughArgumentsException, DateTimeParseException { | ||
String info = fullCommand.substring(8).trim(); | ||
if (info.isEmpty()) { | ||
throw(new TaskNoDescriptionException("OOPS!!! The description of a deadline cannot be empty.")); |
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 have noticed that you put parentheses around new thrown exceptions. I don't think that is against the coding standards, but perhaps leave a space between throw
and (
. This is because throw
is a Java reserved word, so should be followed by a white space.
public static Command parse(String fullCommand) { | ||
String[] commandParts = fullCommand.split(" ", 2); | ||
String commandHeader = commandParts[0]; | ||
switch (commandHeader) { |
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 want to unindent switch cases.
@@ -0,0 +1,28 @@ | |||
package duke.commands; | |||
|
|||
/** This is a command to add a task to Duke |
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.
Can put /**
on separate line
src/main/java/duke/Parser.java
Outdated
import java.time.format.DateTimeParseException; | ||
|
||
/** | ||
* This is a class responsible for parsing the inputs given by user |
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 put punctuation behind each parameter description. I have noticed this in other places too.
src/main/java/duke/tasks/Event.java
Outdated
public Event(String description, LocalDate from, LocalDate to, boolean isDone) { | ||
super(description, isDone); | ||
this.from = from; | ||
this.to= to; |
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 a space around the =
?
public abstract boolean isExit(); | ||
|
||
public abstract String execute(TaskList taskList, Ui ui, Storage storage); | ||
} |
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 used OOP for the commands.
public static Command parse(String fullCommand) { | ||
String[] commandParts = fullCommand.split(" ", 2); | ||
String commandHeader = commandParts[0]; | ||
switch (commandHeader) { |
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 you could think about using enums for the command types?
public static Command parse(String fullCommand) { | ||
String[] commandParts = fullCommand.split(" ", 2); | ||
String commandHeader = commandParts[0]; | ||
switch (commandHeader) { |
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 you could unindent switch cases?
src/test/java/duke/ParserTest.java
Outdated
Command c = Parser.parse("deadline grocery /by 19/10/2023"); | ||
LocalDate date = LocalDate.parse("19/10/2023", DateTimeFormatter.ofPattern("d/MM/yyyy")); | ||
AddDeadlineCommand test = new AddDeadlineCommand("grocery", date); | ||
//AddTodoCommand test = new AddTodoCommand("grocery"); |
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 unused code can be deleted
src/main/java/duke/tasks/Todo.java
Outdated
} | ||
|
||
Todo t = (Todo) o; | ||
if (this.description.equals(t.description)) { |
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 minor nit but perhaps this boolean value could be returned directly.
*/ | ||
public TaskList(ArrayList<Task> tasks) { | ||
this.tasks = tasks; | ||
this.counter = tasks.size(); |
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 could name counter
as size
for clearer naming?
Code has some parts that does not follow coding standards. Changing the code to follow coding standards will help improve readability and quality of the code.
Improve code quality
Add assertions
Duke
Duke helps you to keep track of upcoming tasks that you might have. It's
easyVERY EASY to useYou just need to
And it is FREE
Features include:
Here's the
main
method of Duke: