[Mark Lin] iP#66
Conversation
woodenclock
left a comment
woodenclock
left a comment
There was a problem hiding this comment.
LGTM,
Appropriate use of indentation for the wrapped lines.
Good use of the if-else structure, as stated in the coding standards.
lctxct
left a comment
lctxct
left a comment
There was a problem hiding this comment.
Good job, naming was done well and the code was easy to read.
exetr
left a comment
exetr
left a comment
There was a problem hiding this comment.
Great progress on your iP so far, I have left some comments for potential areas of improvement, especially regarding code quality :)
| out = "List is empty."; | ||
| } | ||
| for (int i = 0; i < listAsArray.length; i++) { | ||
| out += (i == 0 ? "" : "\n") + (i + 1) + ". " + listAsArray[i].getListText(); |
There was a problem hiding this comment.
Try to avoid complicated expressions such as this, consider especially the usage of ternary operators in such contexts, as more complicated comparisons may affect the code readability.
| switch (line[0]) { | ||
| case "T": { | ||
| Task task = new Todo(taskName); | ||
| task.setIsComplete(isMark); | ||
| list.add(task); | ||
| break; | ||
| } | ||
| case "D": { | ||
| String by = line[3]; | ||
| Task task = new Deadline(taskName, by); | ||
| task.setIsComplete(isMark); | ||
| list.add(task); | ||
| break; | ||
| } | ||
| case "E": { | ||
| String from = line[3].split("-")[0]; | ||
| String to = line[3].split("-")[1]; | ||
| Task task = new Event(taskName, from, to); | ||
| task.setIsComplete(isMark); | ||
| list.add(task); | ||
| break; |
There was a problem hiding this comment.
Consider further abstracting out the logic in this portion of the code to avoid deep nesting
| String from = line[3].split("-")[0]; | ||
| String to = line[3].split("-")[1]; |
There was a problem hiding this comment.
Try to avoid usage of magic literals, especially in larger projects, in this case, consider assigning your delimiter to a variable, then referencing when needed.
| throw new DukeException("The description of " + command + " cannot be empty."); | ||
| } | ||
| return message.substring(command.length(), | ||
| message.contains("/") ? message.indexOf("/") : message.length()).trim(); |
There was a problem hiding this comment.
Similarly, keep aware of the usage of magic literals.
4 participants
No description provided.