[Janice Toh] iP #66
[Janice Toh] iP #66
Conversation
…t type for task tracking
There was a problem hiding this comment.
Overall, I could not find any coding standard violations, so great job on that! 💯
The code was also easy to understand and it would be even better if you could apply SLAP and shorten some of the methods in some of the parts that I have highlighted. Keep up the good work and all the best for your iP 👍
[Also took part in this review: @JLoh579 ]
src/main/java/Deadline.java
Outdated
| protected String by; | ||
|
|
||
| public Deadline (String description, String by){ | ||
| super(description, "D"); |
There was a problem hiding this comment.
I was thinking perhaps you can consider defining a variable that corresponds to "D"? It might be better to not use a magic string like "D" that does not explain anything about its meaning
src/main/java/Jan.java
Outdated
|
|
||
| public class Jan { | ||
|
|
||
| public static final int MAXIMUM_TASK = 100; |
There was a problem hiding this comment.
I really like how you defined a constant MAXIMUM_TASK instead of using a magic number! 👍
src/main/java/Jan.java
Outdated
| public static void executeInstruction(String command, Task[] Tasks, String commandDescription){ | ||
| System.out.println("____________________________________________________________"); | ||
| switch(command) { | ||
| case "greet": | ||
| System.out.println("Hello! I'm Jan\n" + " What can I do for you?"); | ||
| break; | ||
| case "bye": | ||
| System.out.println("Bye. Hope to see you again soon!"); | ||
| break; | ||
| case "list": | ||
| System.out.println("Here are the tasks in your list:"); | ||
| for (int i = 0; i < Task.getTotalTask(); i++) { | ||
| System.out.println(Tasks[i].getTaskNum() + ". " + Tasks[i]); | ||
| } | ||
| break; | ||
| case "done": | ||
| int taskNum = Integer.parseInt(commandDescription) - 1; | ||
| if (taskNum < Task.getTotalTask()) { | ||
| System.out.println("Nice! I've marked this task as done: "); | ||
| Tasks[taskNum].setDone(true); | ||
| System.out.println(Tasks[taskNum]); | ||
| }else{ | ||
| System.out.println("Jan cannot find this task"); | ||
| } | ||
| break; | ||
| case "todo": | ||
| Todo todo = new Todo(commandDescription,"T"); | ||
| System.out.println("Got it. I've added this task:\n " + todo); | ||
| printCurrentTaskCount(); | ||
| Tasks[Task.getTotalTask() - 1] = todo; | ||
| break; | ||
| case "deadline": | ||
| String[] deadlineDetails = commandDescription.split("/by"); | ||
| Deadline deadline = new Deadline(deadlineDetails[0],deadlineDetails[1]); | ||
| System.out.println("Got it. I've added this task:\n " + deadline); | ||
| printCurrentTaskCount(); | ||
| Tasks[Task.getTotalTask() - 1] = deadline; | ||
| break; | ||
| case "event": | ||
| String[] eventDetails = commandDescription.split("/at"); | ||
| Event event = new Event(eventDetails[0],eventDetails[1]); | ||
| System.out.println("Got it. I've added this task:\n " + event); | ||
| printCurrentTaskCount(); | ||
| Tasks[Task.getTotalTask() - 1] = event; | ||
| break; | ||
| default: | ||
| System.out.println("Duke cannot understand your command.\n"); | ||
| break; | ||
| } | ||
| System.out.println("____________________________________________________________"); | ||
| } |
There was a problem hiding this comment.
Perhaps you can consider shortening this method since it is about 50 lines of code? Maybe you can consider applying refactoring, especially extracting the methods for each "case"?
src/main/java/Jan.java
Outdated
| case "todo": | ||
| Todo todo = new Todo(commandDescription,"T"); | ||
| System.out.println("Got it. I've added this task:\n " + todo); | ||
| printCurrentTaskCount(); | ||
| Tasks[Task.getTotalTask() - 1] = todo; | ||
| break; | ||
| case "deadline": | ||
| String[] deadlineDetails = commandDescription.split("/by"); | ||
| Deadline deadline = new Deadline(deadlineDetails[0],deadlineDetails[1]); | ||
| System.out.println("Got it. I've added this task:\n " + deadline); | ||
| printCurrentTaskCount(); | ||
| Tasks[Task.getTotalTask() - 1] = deadline; | ||
| break; | ||
| case "event": | ||
| String[] eventDetails = commandDescription.split("/at"); | ||
| Event event = new Event(eventDetails[0],eventDetails[1]); | ||
| System.out.println("Got it. I've added this task:\n " + event); | ||
| printCurrentTaskCount(); | ||
| Tasks[Task.getTotalTask() - 1] = event; | ||
| break; |
There was a problem hiding this comment.
Perhaps you can consider applying SLAP to the blocks of code under "todo", "deadline" and "event". For example, line 35 and lines 33-34 are not in the same level of abstraction, one is a function call whereas the other is performing a computation directly.
No description provided.