Open
Conversation
added 2 commits
January 31, 2025 16:14
Isaaclks7
reviewed
Feb 6, 2025
Isaaclks7
left a comment
Isaaclks7
left a comment
There was a problem hiding this comment.
Overall good naming conventions and relevant spacings. However, you may want to place your different classes into separate files for better organisation.
| @@ -1,3 +1,81 @@ | |||
| import java.util.Scanner; | |||
|
|
|||
| abstract class Task { | |||
There was a problem hiding this comment.
Correct naming convention of your class in PascalCase.
Comment on lines
+12
to
+18
| public void markAsDone() { | ||
| this.isDone = true; | ||
| } | ||
|
|
||
| public void unmarkAsDone() { | ||
| this.isDone = false; | ||
| } |
src/main/java/Lys.java
Outdated
Comment on lines
+101
to
+157
| if (input.equalsIgnoreCase("bye")) { | ||
| System.out.println("____________________________________________________________"); | ||
| System.out.println("Bye. Hope to see you again soon!"); | ||
| System.out.println("____________________________________________________________"); | ||
| break; | ||
| } else if (input.equalsIgnoreCase("list")) { | ||
| System.out.println("____________________________________________________________"); | ||
| System.out.println("Here are the tasks in your list:"); | ||
| for (int i = 0; i < taskCount; i++) { | ||
| System.out.println((i + 1) + "." + tasks[i]); | ||
| } | ||
| System.out.println("____________________________________________________________"); | ||
| } else if (words[0].equalsIgnoreCase("mark")) { | ||
| int index = Integer.parseInt(words[1]) - 1; | ||
| if (index >= 0 && index < taskCount) { | ||
| tasks[index].markAsDone(); | ||
| System.out.println("____________________________________________________________"); | ||
| System.out.println("Nice! I've marked this task as done:"); | ||
| System.out.println(" " + tasks[index]); | ||
| System.out.println("____________________________________________________________"); | ||
| } | ||
| } else if (words[0].equalsIgnoreCase("unmark")) { | ||
| int index = Integer.parseInt(words[1]) - 1; | ||
| if (index >= 0 && index < taskCount) { | ||
| tasks[index].unmarkAsDone(); | ||
| System.out.println("____________________________________________________________"); | ||
| System.out.println("OK, I've marked this task as not done yet:"); | ||
| System.out.println(" " + tasks[index]); | ||
| System.out.println("____________________________________________________________"); | ||
| } | ||
| } else if (words[0].equalsIgnoreCase("todo")) { | ||
| tasks[taskCount] = new ToDo(words[1]); | ||
| taskCount++; | ||
| System.out.println("____________________________________________________________"); | ||
| System.out.println("Got it. I've added this task:"); | ||
| System.out.println(" " + tasks[taskCount - 1]); | ||
| System.out.println("Now you have " + taskCount + " tasks in the list."); | ||
| System.out.println("____________________________________________________________"); | ||
| } else if (words[0].equalsIgnoreCase("deadline")) { | ||
| String[] parts = words[1].split(" /by "); | ||
| tasks[taskCount] = new Deadline(parts[0], parts[1]); | ||
| taskCount++; | ||
| System.out.println("____________________________________________________________"); | ||
| System.out.println("Got it. I've added this task:"); | ||
| System.out.println(" " + tasks[taskCount - 1]); | ||
| System.out.println("Now you have " + taskCount + " tasks in the list."); | ||
| System.out.println("____________________________________________________________"); | ||
| } else if (words[0].equalsIgnoreCase("event")) { | ||
| String[] parts = words[1].split(" /from | /to "); | ||
| tasks[taskCount] = new Event(parts[0], parts[1], parts[2]); | ||
| taskCount++; | ||
| System.out.println("____________________________________________________________"); | ||
| System.out.println("Got it. I've added this task:"); | ||
| System.out.println(" " + tasks[taskCount - 1]); | ||
| System.out.println("Now you have " + taskCount + " tasks in the list."); | ||
| System.out.println("____________________________________________________________"); | ||
| } |
There was a problem hiding this comment.
Could consider using switch-case instead of multiple if-else statements
src/main/java/Lys.java
Outdated
Comment on lines
+109
to
+110
| for (int i = 0; i < taskCount; i++) { | ||
| System.out.println((i + 1) + "." + tasks[i]); |
There was a problem hiding this comment.
Could rename i to taskIndex for readability. Additionally, you can start from i = 1 instead of i = 0.
added 2 commits
February 13, 2025 10:08
Change the if-else statements into switch case to improve readability.
xenthm
reviewed
Feb 22, 2025
src/main/java/Lys.java
Outdated
Comment on lines
+103
to
+109
| switch (command) { | ||
| case "bye": | ||
| System.out.println("____________________________________________________________"); | ||
| System.out.println("Bye. Hope to see you again soon!"); | ||
| System.out.println("____________________________________________________________"); | ||
| scanner.close(); | ||
| return; |
There was a problem hiding this comment.
Do try to use the formatting for switch statements outlined in the Java coding standards for this course.
Comment on lines
+124
to
+141
| case "mark": | ||
| case "unmark": | ||
| int index = Integer.parseInt(words[1]) - 1; | ||
| if (index < 0 || index >= taskCount) { | ||
| throw new IndexOutOfBoundsException("Invalid task number."); | ||
| } | ||
| if (command.equals("mark")) { | ||
| tasks[index].markAsDone(); | ||
| System.out.println("____________________________________________________________"); | ||
| System.out.println("Nice! I've marked this task as done:"); | ||
| } else { | ||
| tasks[index].unmarkAsDone(); | ||
| System.out.println("____________________________________________________________"); | ||
| System.out.println("OK, I've marked this task as not done yet:"); | ||
| } | ||
| System.out.println(" " + tasks[index]); | ||
| System.out.println("____________________________________________________________"); | ||
| break; |
There was a problem hiding this comment.
Good job realising that the mark and unmark commands are very similar. Consider extracting the logic out to another method so that you can avoid placing the if-else block within the case block for mark and unmark, which basically does the same thing.
Comment on lines
+111
to
+122
| case "list": | ||
| System.out.println("____________________________________________________________"); | ||
| if (taskCount == 0) { | ||
| System.out.println("Your task list is empty."); | ||
| } else { | ||
| System.out.println("Here are the tasks in your list:"); | ||
| for (int i = 0; i < taskCount; i++) { | ||
| System.out.println((i + 1) + "." + tasks[i]); | ||
| } | ||
| } | ||
| System.out.println("____________________________________________________________"); | ||
| break; |
There was a problem hiding this comment.
Perhaps you can extract some of the logic in your cases out into methods in order to reduce deep nesting and prevent "arrowhead code" from developing.
A-UserGuide
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.