[Izdiyad] iP - #184
Conversation
daknam2001
left a comment
daknam2001
left a comment
There was a problem hiding this comment.
good job with following coding standards overall!
| public class Deadline extends Task { | ||
| protected String by; | ||
|
|
||
| public Deadline(String description, String by) { |
There was a problem hiding this comment.
Well done with naming classes in PascalCase
| private static int taskCount; | ||
|
|
||
| // prints a string within two horizontal lines, @param is string to be printed | ||
| public static void printWithLines(String text) { |
There was a problem hiding this comment.
Good job with writing method names in camelCase
| String inputCommand = input.trim().split(" ")[0]; | ||
| String inputData = input.replaceFirst(inputCommand, "").trim(); | ||
|
|
||
| switch (inputCommand){ |
There was a problem hiding this comment.
good job with the indentation of switch cases
| taskCount++; | ||
| } | ||
|
|
||
| public static void listTasks() { |
There was a problem hiding this comment.
egyptian style brackets used, well done!
|
|
||
| public static void addTask(String task) { | ||
|
|
||
| if (task.startsWith("todo")) { |
There was a problem hiding this comment.
curly braces used for all if-else statements, well done
| public class Duke { | ||
| public static void main(String[] args) { | ||
| private static Task[] tasks; | ||
| private static int taskCount; |
There was a problem hiding this comment.
variable names are in camelCase, nice job
| printWithLines(byeMessage); | ||
| } | ||
|
|
||
| public static void addTask(String task) { |
There was a problem hiding this comment.
Add comments on how the method works and explain the parameters
|
|
||
| public static void addTask(String task) { | ||
|
|
||
| if (task.startsWith("todo")) { |
There was a problem hiding this comment.
I think can work on the readability of the code by creating a method for each else if statement.
| + chosenTask.description); | ||
| } | ||
|
|
||
| public static void selectCommand(String input) { |
There was a problem hiding this comment.
Recommend to refractor code to include a task manager to handle all the task events instead of all in duke.java
| protected String description; | ||
| protected boolean isDone; | ||
|
|
||
| public Task(String description) { |
There was a problem hiding this comment.
Good to include comments to explain what Task is
Give users a way to find a task by searching for a keyword.
# Conflicts: # src/main/java/duke/Parser.java # src/main/java/duke/TaskList.java
Samuel787
left a comment
Samuel787
left a comment
There was a problem hiding this comment.
Your code follows good OOP structure. Good code style and quality is mostly observed. LGTM! Good job!
| ui.printWithLines(e.getMessage()); | ||
| } | ||
|
|
||
| // Gets the next command entered |
There was a problem hiding this comment.
The code is well structured and easy to understand what the code is doing here. Hence, this comment seems redundant. You may want to consider removing it :)
| public Command addTask(String input) throws DukeException { | ||
|
|
||
| if (input.startsWith(TODO)) { | ||
| if (input.substring(4).isEmpty()) { |
| trimInput(input); | ||
|
|
||
| } else if (input.startsWith(DEADLINE)) { | ||
| if (!input.contains("/by")) { |
There was a problem hiding this comment.
You can also consider making "/by" a constant just like DEADLINE
| Command commandFromInput = null; | ||
|
|
||
| switch (trimmedInput){ | ||
| case TODO: case DEADLINE: case EVENT: |
There was a problem hiding this comment.
When you have multiple cases and you want the control to cascade down, you can still follow the same recommended code style to have each case in a new line.
|
|
||
| protected Ui ui; | ||
| protected TaskList tasks; | ||
| private int targetIndex = -1; |
There was a problem hiding this comment.
This private variable is declared in this abstract class but not used. Do you need this variable?
4 participants
No description provided.