[kestryix] iP#96
Conversation
|
|
||
| case "unmark": | ||
| String[] split = input.split(" "); | ||
| int idx = Integer.parseInt(split[1]) - 1; |
There was a problem hiding this comment.
Consider using more descriptive variable names, such as "index" instead of "idx", for better readability.
| Scanner scanner = new Scanner(System.in); | ||
| String input = scanner.nextLine(); | ||
|
|
||
| // if (Objects.equals(input.toLowerCase(), "bye")) { |
There was a problem hiding this comment.
Consider removing the commented-out code if it is no longer needed for clarity and to keep the codebase clean.
| System.out.println("--------------------------------------------"); | ||
| System.out.println("Hello, I'm TulipTask"); | ||
| System.out.println("What can I do for you today?"); | ||
| System.out.println("--------------------------------------------"); |
There was a problem hiding this comment.
Consider extracting the line as a public static final constant to improve maintainability and avoid duplication.
| import java.util.Objects; | ||
| import java.util.Scanner; | ||
|
|
||
| public class TulipTask { |
There was a problem hiding this comment.
Consider renaming the class to "Tulip" as "TulipTask" suggests it might inherit from a "Task" class, which can be misleading.
| @@ -0,0 +1,21 @@ | |||
| public class Task { | |||
| protected String description; | |||
| protected boolean isDone; | |||
There was a problem hiding this comment.
The boolean variable name "isDone" is well-named, as it clearly indicates its purpose and state.
louisjoety
left a comment
louisjoety
left a comment
There was a problem hiding this comment.
Good job overall, just a few nits to fix. Keep it up @kestryix! 🚀
| switch(command) { | ||
| case "list": | ||
| listTasks(list); | ||
| break; | ||
|
|
||
| case "mark": | ||
| String[] splitStr = input.split(" "); | ||
| int index = Integer.parseInt(splitStr[1]) - 1; | ||
| markAsDone(list.get(index)); | ||
| break; | ||
|
|
||
| case "unmark": | ||
| String[] split = input.split(" "); | ||
| int idx = Integer.parseInt(split[1]) - 1; |
There was a problem hiding this comment.
Maybe avoid different levels of abstraction here (i.e.listTasks(list) and String[] splitStr = input.split(" ") and int index = Integer.parseInt(splitStr[1]) - 1
| public String getStatusIcon() { | ||
| return (isDone ? "X" : " "); // mark done task with X | ||
| } |
There was a problem hiding this comment.
Maybe you can make the code more explicit here instead of using ternary if operator?
| import java.util.Objects; | ||
| import java.util.Scanner; | ||
|
|
||
| public class TulipTask { |
There was a problem hiding this comment.
Good class naming as it is not too long and not short. Nice work!
| System.out.println("Okay, I have marked this task as not done: \n" + " [" + task.getStatusIcon() + "] " + task.description); | ||
| System.out.println("--------------------------------------------"); | ||
| } | ||
| } |
There was a problem hiding this comment.
You may consider adding comment statements here to explain to other engineers what and why you are doing this.
| System.out.println("--------------------------------------------"); | ||
| System.out.println("Great job! I have marked this task as done: \n" + " [" + task.getStatusIcon() + "] " + task.description); | ||
| System.out.println("--------------------------------------------"); | ||
| } |
There was a problem hiding this comment.
I think you need to fix the spacings here.
thomasjlalba
left a comment
thomasjlalba
left a comment
There was a problem hiding this comment.
Great work! Just some comments mainly on the standards to use in this course.
| String type = line.substring(1, 2); | ||
| boolean completed = line.charAt(4) == 'X'; | ||
|
|
||
| switch (type) { |
There was a problem hiding this comment.
For your switch statements, maybe you could follow the style mentioned here.
|
|
||
| public class TulipTask { | ||
| public static void main(String[] args) throws TulipTaskException.InvalidTaskDescriptionException, TulipTaskException.InvalidEndDateException, TulipTaskException.InvalidStartDateException, TulipTaskException.InvalidDeadlineException, TulipTaskException.InvalidTaskIndexException { | ||
| String logo = " \n" + |
There was a problem hiding this comment.
Excellent job on storing these strings and not using them as Magic strings!
| String command = commandArguments.get(InputParser.COMMAND); | ||
|
|
||
| switch (command) { | ||
| case "list": |
There was a problem hiding this comment.
Maybe you could refactor to avoid magic strings!
| import java.util.Scanner; | ||
|
|
||
| public class TulipTask { | ||
| public static void main(String[] args) throws TulipTaskException.InvalidTaskDescriptionException, TulipTaskException.InvalidEndDateException, TulipTaskException.InvalidStartDateException, TulipTaskException.InvalidDeadlineException, TulipTaskException.InvalidTaskIndexException { |
There was a problem hiding this comment.
The main method seems quite long, maybe you could refactor it to several smaller methods!
|
|
||
| public void parseTask(String line) { | ||
| String type = line.substring(1, 2); | ||
| boolean completed = line.charAt(4) == 'X'; |
There was a problem hiding this comment.
Maybe the naming of your boolean can be improved to sound like a boolean!
4 participants
No description provided.