[Maliha Haque] iP #427
[Maliha Haque] iP #427
Conversation
There was a problem hiding this comment.
You are making good progress in catching up. Aside from a few minor coding violations and some long pieces of code, everything else look fine. Before you progress into later weeks of the IP, it would be good to install the checkstyle scripts from week 4 to help you check coding standard violation. Aside from that, configuring your IDE to fit the coding standard is available in week 2 or 3 so perhaps you can skim through and configure it now.
|
|
||
| private static void printList() { | ||
| System.out.println("Here are the tasks in your list:"); | ||
| for (int i=1; i<=list.size(); i++) { |
There was a problem hiding this comment.
Your operators (e.g. "=", "<=", "-") should be surrounded by space on both sides. This minor issues reappear a few times in other parts of your code(e.g. addTask(String) method).
| unmarkTask(message); | ||
| } else if (firstWord.equals("delete")) { | ||
| deleteTask(message); | ||
| } else if (firstWord.equals("todo") || firstWord.equals("deadline") || firstWord.equals("event")){ |
There was a problem hiding this comment.
Perhaps we can split into more if-else statements and make each task creation its own function?
| @@ -0,0 +1,5 @@ | |||
| public class LaraException extends Exception{ | |||
There was a problem hiding this comment.
{ should have a white space preceding it.
| @@ -0,0 +1,34 @@ | |||
| import java.util.ArrayList; | |||
|
|
|||
| public class Task { | |||
There was a problem hiding this comment.
Consider putting your object classes into packages (naming for packages need to be all lowercase). It is a week 3 IP requirement but it would be good to start doing it in earlier branches.
| public class Task { | ||
| protected String description; | ||
| protected boolean isDone; | ||
| protected static int taskCounter=0; |
There was a problem hiding this comment.
Perhaps we can remove this if we do not plan to use it?
| System.out.println(event); | ||
| } | ||
| System.out.println("Now you have " + list.size() + " tasks in the list."); | ||
| } catch (LaraException e) { |
There was a problem hiding this comment.
Do we need a try-catch for every task-related function call if we already have a try-catch in your if-else statements?
| System.out.println("OK, I've marked this task as not done yet:"); | ||
| System.out.println("[ ] " + list.get(taskNumber).getDescription()); | ||
| } catch (LaraException e) { | ||
| System.out.println(e.getMessage()); |
There was a problem hiding this comment.
This line should have another 4-space indentation. some of the spacing for the "}" brackets below also seem a bit inconsistent, can make use of IDEs which automatically adjust the indentation for you so you don't have to manually check :)
No description provided.