-
Notifications
You must be signed in to change notification settings - Fork 362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[bryanongjx] iP #379
base: master
Are you sure you want to change the base?
[bryanongjx] iP #379
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall no major code style violations. After tidying up the long lines, you are good to go!
src/main/java/Duke.java
Outdated
@@ -1,10 +1,83 @@ | |||
import java.util.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the coding standard, you may want to list all imports explicitly as they make the class easier to read 😀
src/main/java/Duke.java
Outdated
System.out.println("Hello! I'm Duke\n"); | ||
System.out.println("What can I do for you?\n"); | ||
|
||
List<Task> lst = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice naming, easy to understand in subsequent code 👍
src/main/java/Duke.java
Outdated
String details = actionAndDetails[1]; | ||
Todo newTodo = new Todo(details); | ||
lst.add(newTodo); | ||
System.out.println("Got it. I've added this task:" + '\n' + newTodo + '\n' + "Now you have " + lst.size() + " tasks in the list"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line may be exceeding the 120 character limit, perhaps you would like to double check!
src/main/java/Duke.java
Outdated
lst.remove(taskNum - 1); | ||
System.out.println("Noted. I've removed this task:" + '\n' + currTask + '\n' + "Now you have " + lst.size() + " tasks in the list"); | ||
}else { | ||
throw new DukeException("OOPS!!! I'm sorry, but I don't know what that means :-("); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cute emoji 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM. Good job on completing up to Level 6. All the best for the upcoming weeks.
src/main/java/Event.java
Outdated
String From; | ||
String To; | ||
public Event(String description, String From, String To) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps, the naming of the fields can follow the norm of the 'description' field with small letters instead of capitalised ones. I think as long as you are consistent, it should be fine
src/main/java/Duke.java
Outdated
if (action.equals("bye")) { | ||
System.out.println("Bye. Hope to see you again soon!"); | ||
return; | ||
} else if (action.equals("list")) { // list | ||
System.out.println("Here are the tasks in your list:"); | ||
for (int i = 1; i <= lst.size(); i++) { | ||
System.out.println(i + ". " + lst.get(i - 1)); | ||
} | ||
} else if (action.equals("mark")) { | ||
String details = inputArr[1]; | ||
int taskNum = Integer.parseInt(details); | ||
Task currTask = lst.get(taskNum - 1); | ||
currTask.mark(); | ||
System.out.println("Nice! I've marked this task as done" + '\n' + currTask); | ||
} else if (action.equals("unmark")) { | ||
String details = inputArr[1]; | ||
int taskNum = Integer.parseInt(details); | ||
Task currTask = lst.get(taskNum - 1); | ||
currTask.unMark(); | ||
System.out.println("Nice! I've marked this task as not done yet" + '\n' + currTask); | ||
} else if (action.equals("todo")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps, using a switch-case statement would make it slightly more readable.
This reverts commit cf92c42.
# Conflicts: # data/duke.txt # src/main/java/Duke.java
add assertions to TaskList methods
Tick Pro
"Tick OP" - every salty brawl stars player
Tick Pro frees your mind of having to remember things you need to do. It's,
FASTSUPER FAST to learnSetting it up
and it's FREE!! 🤑
Features:
If you are a java programmer, you can use it to practice java too. Here's
the main method: