[Dexter Hoon] iP#68
Conversation
| //unmark tasks as undone | ||
| String[] words = userInput.split(" "); | ||
|
|
||
| tasks[Integer.parseInt(words[1])].setDone(false); |
There was a problem hiding this comment.
Could you rename this line as a variable, seeing as you use it later in your code?
There was a problem hiding this comment.
Thanks for the suggestion... I think it could be helpful to do this as it can make my code neater :D
| System.out.println("Ok, I've marked this task as not done yet:"); | ||
| System.out.println("\t[" + taskList[Integer.parseInt(words[1])].getStatusIcon() + "] " + | ||
| taskList[Integer.parseInt(words[1])].getDescription()); | ||
| System.out.println("\t[" + tasks[Integer.parseInt(words[1])].getStatusIcon() + "] " + |
There was a problem hiding this comment.
Maybe making some variables statement could make this less verbose
There was a problem hiding this comment.
I agree... will probably do that since it does look very complex and hard to understand what is going on
skylee03
left a comment
skylee03
left a comment
There was a problem hiding this comment.
The code quality is very high!
| System.out.println("\t[" + tasks[Integer.parseInt(words[1])].getStatusIcon() + "] " + | ||
| tasks[Integer.parseInt(words[1])].getDescription()); |
There was a problem hiding this comment.
Try to avoid complicated expressions.
There was a problem hiding this comment.
Yeah very true... I will follow what someone else suggested by using more variables to simplify the statement
| System.out.println("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"); | ||
| } | ||
|
|
||
| public static void main(String[] args) { |
There was a problem hiding this comment.
Try to avoid long methods. You can try to extract part of the code into other methods.
There was a problem hiding this comment.
Alright sure I can try to break the long method into smaller methods to handle each of the commands which can make it easier to debug and organise my code
There was a problem hiding this comment.
Good job on the PR! Here are some feedbacks:
Perhaps you would like to consider reviewing the variable names as I noticed that there was variable shadowing (i.e. currentTasksIndex is used as an attribute name as well as a parameter in the function).
Also, perhaps do you want to consider the standardisation of the use of variables and magic strings? I noticed that you declared variables for the error messages but not for the other messages and hence magic strings still exists.
Do you also want to consider using attributes for the tasks array and user input because it is the class' property and many methods will use them over and over again?
Finally, do you want to relook at the code readability as I noticed that many attributes and methods were not spaced consistently, making it difficult to read. Do you want to have a look at switch case or String.formatf
| public static void printTaskAddedMessage(Task[] tasks, int currentTasksIndex) { | ||
| System.out.println("added: " + tasks[currentTasksIndex]); | ||
| System.out.println("Now you have " + currentTasksIndex + " in the list."); | ||
| } |
There was a problem hiding this comment.
Do you want to consider changing the variable name for the currentTasksIndex as I noticed that it shares the same name as the class attribute and might be ambiguous (have to be more careful in the future)?
| this.description = description; | ||
| } | ||
|
|
||
| public boolean getDone() { |
There was a problem hiding this comment.
Perhaps you would like to use a more boolean name for this method
DextheChik3n
force-pushed
the
master
branch
from
1bc81a4 to
4194ba3
Compare
Add Date parsing for deadline and event tasks
# Conflicts: # src/main/java/alan/parser/Parser.java
# Conflicts: # src/main/java/alan/parser/Parser.java
Add find task functionality
# Conflicts: # src/main/java/alan/data/exception/AlanException.java # src/main/java/alan/parser/Parser.java # src/main/java/alan/storage/Storage.java # src/main/java/alan/ui/Ui.java
Add JavaDoc comments
4 participants
No description provided.