[Wang Zihan] iP - #94
Conversation
| //Level 3: Mark as Done | ||
| Scanner in = new Scanner(System.in); | ||
| String userInput; | ||
| Task[] taskList = new Task[100]; //TODO: Remove magic literals, refactor and extract constant |
| } | ||
|
|
||
| public String getStatusIcon() { | ||
| return (isDone ? "\u2713" : "\u2718"); |
There was a problem hiding this comment.
Arbitrary unicode can be replaced by a more expressive constant instead
| lineBreak(); | ||
| } | ||
|
|
||
| public static void lineBreak() { |
There was a problem hiding this comment.
I like the use of method extraction for line breaks
There was a problem hiding this comment.
Agree with the good use of lineBreak() on method extraction! Perhaps can create a class for all the miscellaneous output like this.
| String by = userInput.substring(dividerPosition + 4); | ||
| taskList[listCount++] = new Deadline(userInput.substring(9, dividerPosition - 1), by); | ||
| System.out.println(" " + taskList[listCount - 1].toString()); | ||
| System.out.println("Now you have " + listCount + " tasks in the list."); |
There was a problem hiding this comment.
It would be good practice to keep the indentation for wrapped lines to: 8 spaces more than the parent line.
|
|
||
| public static void exitProgram() { | ||
| lineBreak(); | ||
| System.exit(0); |
vaiish371
left a comment
vaiish371
left a comment
There was a problem hiding this comment.
Overall, I found your code very clean and it seems to have followed 99% of the coding standard. I could hardly find any errors, so good job!
liping-eng
left a comment
liping-eng
left a comment
There was a problem hiding this comment.
Overall, good use of camelCase for variables and methods. Good use K&R style brackets.
| lineBreak(); | ||
| } | ||
|
|
||
| public static void lineBreak() { |
There was a problem hiding this comment.
Agree with the good use of lineBreak() on method extraction! Perhaps can create a class for all the miscellaneous output like this.
| Scanner in = new Scanner(System.in); | ||
| String userInput; | ||
| Task[] taskList = new Task[100]; | ||
| Task[] taskList = new Task[100]; //TODO: Remove magic literals, refactor and extract constant |
There was a problem hiding this comment.
Perhaps change "taskList" to "tasks" (plural form) as it is representing a collection of objects.
|
|
||
| public static final int MAX_TASK = 100; | ||
|
|
||
| static Task[] taskList = new Task[MAX_TASK]; |
There was a problem hiding this comment.
Perhaps you can use plural variable name for representing the collection of Task objects. Even though taskList is quite clear and can be easily understood, use of plural word like "tasks" could better convey the meaning.
vaiish371
left a comment
vaiish371
left a comment
There was a problem hiding this comment.
I have left you few more comments, that have to do with very minor errors - naming convention. Apart that, the code looks really clean!
# Conflicts: # src/main/java/duke/Duke.java
|
|
||
| @Override | ||
| public String toString() { | ||
| return "[D]" + super.toString() + " (by: " + by + ")"; |
There was a problem hiding this comment.
maybe you should explore the use of constants here in order to reduce the need of magic strings.
| greet(); | ||
| loadFile(); | ||
| while (true) { | ||
| String userInput = SCANNER.nextLine(); | ||
| executeCommand(userInput); | ||
| } | ||
| } |
| int dividerPosition; | ||
| char taskType = line.charAt(1); | ||
| switch (taskType) { | ||
| case 'T': |
There was a problem hiding this comment.
to improve code readability, maybe we should use enums here?
| default: | ||
| System.out.println("Unknown task type found: " + taskType); | ||
| } | ||
| if (line.charAt(4) == '\u2713') { |
There was a problem hiding this comment.
refer to the magic strings comment.
| char taskType = line.charAt(1); | ||
| switch (taskType) { | ||
| case 'T': | ||
| taskList.add(new Todo(line.substring(7))); |
There was a problem hiding this comment.
what does line.substring(7) stand for? maybe we should assign a variable for it?
Add Dates and Times
# Conflicts: # src/main/java/duke/Tasks/Task.java
Add find command
5 participants
No description provided.