[Isaac] iP - #496
Conversation
vvidday
left a comment
vvidday
left a comment
There was a problem hiding this comment.
Overall, your code looks good and I really like how you organized it nicely into different packages, especially commands! There's a couple of minor code quality / coding standard issues below, but other than that, LGTM!
|
|
||
| import java.time.format.DateTimeFormatter; | ||
|
|
||
| public abstract class Command { |
There was a problem hiding this comment.
I like the use of an abstract class here, as well as your great organization in the commands package!
| } | ||
| } | ||
|
|
||
| public void store(TaskList toStore) { |
There was a problem hiding this comment.
Maybe you could consider writing javadocs for these more complicated public facing methods, to give others a clearer picture of what they do?
|
|
||
| @Override | ||
| public void execute(TaskList taskList, UI ui, Storage storage) { | ||
| String taskDesc = input.substring(5); |
There was a problem hiding this comment.
Perhaps a named constant instead of a magic number would make the code here (and other parts) clearer?
| private final int taskID; | ||
|
|
||
| public DeleteTaskCommand(String input) throws DukeException { | ||
| if (!checkValid(input)) throw new DukeException(" ☹ OOPS!!! Please enter task to delete."); |
There was a problem hiding this comment.
Should the conditional here be wrapped in curly brackets?
| return taskList.size() == 0; | ||
| } | ||
|
|
||
| public int size() { |
There was a problem hiding this comment.
Should this method name be a verb? e.g. getSize()
| if (isEmpty()) { | ||
| throw new DukeException(" ☹ OOPS!!! Seems like your list is empty."); |
There was a problem hiding this comment.
I like that you thought of & handled this edge case!
marcusczh
left a comment
marcusczh
left a comment
There was a problem hiding this comment.
Other than some minor code standard issues, the overall code looks good and is ready to merge! I really like the use of OOP to abstract out the common functionalities as well!
| @@ -0,0 +1,44 @@ | |||
| package Duke.commands; | |||
There was a problem hiding this comment.
You may want to consider changing names representing packages to all lowercase, to conform to the coding standard.
| } | ||
| } | ||
|
|
||
| public void run() { |
There was a problem hiding this comment.
I really like how all the detailed functionalities of Duke.run() are abstracted further into classes.
| @@ -0,0 +1,66 @@ | |||
| package Duke.storage; | |||
|
|
|||
| import Duke.tasks.*; | |||
There was a problem hiding this comment.
You may want to consider avoiding wildcard imports
| import Duke.commands.*; | ||
| import Duke.exceptions.DukeException; | ||
|
|
||
| public class Parser { |
There was a problem hiding this comment.
I like how you abstracted the functionalities here into separate Command classes
|
|
||
| public class MarkTaskCommand extends Command { | ||
|
|
||
| private final int taskID; |
There was a problem hiding this comment.
I think taskID can be written as taskId instead to conform to the camelCase standard
Duke uses terminal to receive inputs and display outputs. This interface is ugly and not user friendly. New GUI was implemented to make Duke appear in chat form, much more friendly.
CodeQuality Branch
Undo has some bugs It does not work as intended after some testing The implementation was changed to fix the bugs
GUi is lacktuster Made several improvements (added profile picture, etc...) Fixed several bugs from previous code
This reverts commit 5f7ef7b.
3 participants
Duke
“Productivity is never an accident. It is always the result of a commitment to excellence, intelligent planning, and focused effort.” – Paul J. Meyer (source)
Duke tracks your tasks for you so your mind is free to focus on the tasks at hand. It's,
quickLIGHTNING FAST in response timeAnd it is FREE!
Features:
If you Java programmer, you can use it to practice Java too. Here's the main method:
public class Main { public static void main(String[] args) { new Duke(data/tasks.txt).run(); } }