[LimHongYao] iP#201
Conversation
Changed Duke's Name
Improved handling of commands by splitting user input Changed to use Tasks classes
LimHongYao
changed the title
khooyourun
left a comment
khooyourun
left a comment
There was a problem hiding this comment.
Overall, the code is readable and the if statements are not deeply nested.
| boolean exit = false; | ||
| while (!exit) { | ||
| String input = (in.nextLine()).trim(); | ||
| String inputCMD, inputItem; |
There was a problem hiding this comment.
Can I check what does CMD mean? If it stands for command. I think it's better to just name it as inputCommand for readibilty.
| inputCMD = input.split(" ", 2)[0]; | ||
| inputItem = input.split(" ", 2)[1]; |
There was a problem hiding this comment.
The numbers like 2, 0, 1 might be considered magic numbers that hinder readability.
| } | ||
| } | ||
| public static void markDone (int index) { | ||
| if (TaskList.get(index).getStatusIcon().equals(" ")) { |
There was a problem hiding this comment.
Is this if statement necessary? I think markDone would change the status icon to "X" independent of what is originally is
| ToDoList.markDone(Integer.parseInt(inputItem)-1); | ||
| System.out.println("Okay I've marked item " + inputItem + " as done:"); | ||
| ToDoList.printItem(Integer.parseInt(inputItem)-1); |
There was a problem hiding this comment.
I like the single level of abstraction applied here. Makes things easy to follow
| System.out.println("Hi it's Anna!\nWhat do you need to do?"); | ||
| Scanner in = new Scanner(System.in); | ||
|
|
||
| boolean exit = false; |
There was a problem hiding this comment.
Preferably name boolean in such a way that implies it is boolean.
There was a problem hiding this comment.
Maybe use isExit to replace exit can imply a boolean
liuziyang020319
left a comment
liuziyang020319
left a comment
There was a problem hiding this comment.
Most the codes follow the coding standard
| System.out.println("Hi it's Anna!\nWhat do you need to do?"); | ||
| Scanner in = new Scanner(System.in); | ||
|
|
||
| boolean exit = false; |
There was a problem hiding this comment.
Maybe use isExit to replace exit can imply a boolean
| import java.util.ArrayList; | ||
|
|
||
| public class ToDoList { | ||
| private static final ArrayList<Task> TaskList = new ArrayList<>(10); |
There was a problem hiding this comment.
Maybe V\variable names need to be in camelCase.
| import java.util.ArrayList; | ||
|
|
||
| public class ToDoList { | ||
| private static final ArrayList<Task> TaskList = new ArrayList<>(10); |
There was a problem hiding this comment.
Maybe variable names need to be in camelCase.
|
|
||
| public class ToDoList { | ||
| private static final ArrayList<Task> TaskList = new ArrayList<>(10); | ||
| private static int NumTasks = 0; |
There was a problem hiding this comment.
Maybe variable names must be in camelCase, same problem.
| import java.util.ArrayList; | ||
|
|
||
| public class ToDoList { | ||
| private static final ArrayList<Task> TaskList = new ArrayList<>(10); |
There was a problem hiding this comment.
Maybe the size of the ArrayList should be a magic number.
added immediate prompt for missing information reworked the terrible code in getStartEnd
| import java.util.Scanner; | ||
|
|
||
| public class Duke { | ||
| static final int COMMAND_INDEX = 0; |
There was a problem hiding this comment.
Good job using constants for your numbers! You could extend that to magic strings as well!
| TaskList.viewList(); | ||
| } | ||
| break; | ||
|
|
There was a problem hiding this comment.
Do avoid leaving the space between each case as per the Java Coding Standards
| public String getTypeIcon() { | ||
| return "NULL"; | ||
| } | ||
| public String taskTypeIcon() { return "[" + getTypeIcon() + "]";} |
There was a problem hiding this comment.
Even though these are simple 1 liners, please do still follow the proper style (aka Egyptian Style).
| import java.util.ArrayList; | ||
|
|
||
| public class TaskList { | ||
| private static final ArrayList<Task> TaskList = new ArrayList<>(10); |
There was a problem hiding this comment.
ArrayList is a type of dynamic array and there is no need to limit your TaskList to a size of 10 only!
| System.out.println(TaskList.get(i).getTask()); | ||
| } | ||
| } | ||
| public static void markDone (int index) { |
There was a problem hiding this comment.
There should not be a space between markDone and (. Do change this for your other methods as well :)
| protected String description; | ||
| protected boolean isDone; | ||
|
|
||
| public Task(String description) { //ok to leave as public? |
There was a problem hiding this comment.
Yes you need to leave it as public. But do remove such comments from your code as it goes against the java coding standard. 👍
fixed minor presentation error in event
introduced some constants introduced file path printout in event of export failure
removed redundanct "add" method in Duke.java
Reformatted event and deadline displays
added search functionality
added parser class
added javadoc documentation
4 participants
Renamed Duke
Added ToDoList functionality (add, mark, unmark, show list)
Given some personality