[Liu Hao] ip #115
[Liu Hao] ip #115
Conversation
src/main/java/Taylor.java
Outdated
| String input = sc.nextLine(); | ||
| List<Task> tasks = new ArrayList<>(); | ||
| while(!input.equals("bye")) { | ||
| if(input.equals("list")) { |
There was a problem hiding this comment.
Is it possible to use a switch-case instead of if-else?
src/main/java/Taylor.java
Outdated
| if(input.equals("list")) { | ||
| System.out.println(line); | ||
| System.out.println("Here are the tasks in your list:"); | ||
| for(int i = 0; i < tasks.size(); i++) { |
There was a problem hiding this comment.
Since tasks.size() is repeated used in all of the loops, perhaps can create a variable to store this value
src/main/java/Taylor.java
Outdated
| if(input.startsWith("mark")){ | ||
| String[] words = input.split(" "); | ||
| int index = Integer.parseInt(words[1])-1; | ||
| if(index<0 || index>=tasks.size()) { |
There was a problem hiding this comment.
Perhaps can place the code for checking in a method to allow easier reading
src/main/java/Taylor.java
Outdated
|
|
||
| String input = sc.nextLine(); | ||
| List<Task> tasks = new ArrayList<>(); | ||
| while(!input.equals("bye")) { |
There was a problem hiding this comment.
For all the printing of the statements, maybe can put it in a method each. For instance, printTodoAdded() and the statement that the todo task have been added will be printed
src/main/java/Taylor.java
Outdated
| int from = input.indexOf("/from"); | ||
| int to = input.indexOf("/to"); | ||
| String description = input.substring(6,from); | ||
| String _from = input.substring(from+6,to); |
There was a problem hiding this comment.
Preferably used another name instead of _from or _to as it is similar as from and to, which can be confusing
src/main/java/Taylor.java
Outdated
| import java.util.*; | ||
|
|
||
| // Main class to implement a task manager called "Taylor" | ||
| public class Taylor { |
There was a problem hiding this comment.
didn't do deep nesting, which is good code quality
src/main/java/Taylor.java
Outdated
| // Initialize a scanner object to take user input | ||
| Scanner sc = new Scanner(System.in); | ||
|
|
||
| // Define a separator line for display purposes |
There was a problem hiding this comment.
appropriate code comments explanations are helpful
src/main/java/Taylor.java
Outdated
| if(input.startsWith("mark")){ | ||
| String[] words = input.split(" "); | ||
| int index = Integer.parseInt(words[1])-1; // Parse task index | ||
| if(index<0 || index>=tasks.size()) { // Check if index is valid |
There was a problem hiding this comment.
error handling for Invalid Input is very useful
src/main/java/Taylor.java
Outdated
| } | ||
|
|
||
| // Handle "todo" command to add a new Todo task | ||
| if(input.startsWith("todo")){ |
There was a problem hiding this comment.
Clear input handling for all the multiple types of commands (list, mark, unmark, todo, event, deadline)
| * @throws IOException If an I/O error occurs while reading the file. | ||
| * @throws TaylorException If the file contains an unknown task type. | ||
| */ | ||
| public ArrayList<Task> load() throws IOException, TaylorException { |
There was a problem hiding this comment.
Perhaps provide a better descriptive name for the method?
This applies to the method names in this file such as "save" as well.
| String description = parts[2].trim(); | ||
|
|
||
| switch (taskType) { | ||
| case "T" -> tasks.add(new Todo(description, isCompleted)); |
There was a problem hiding this comment.
Please follow the structure of switch statements using breaks and colons, instead of arrow.
| * The Taylor class handles user commands and manages task operations. | ||
| */ | ||
| public class Taylor { | ||
| private final Storage storage; |
There was a problem hiding this comment.
Constants should use SCREAMING_SNAKE_CASE instead.
| * @return The next user command input. | ||
| * @throws TaylorException If an unknown command is encountered. | ||
| */ | ||
| private String operate(String input) throws TaylorException { |
There was a problem hiding this comment.
This method is very long, perhaps abstract some details out into new functions (especially for the code in switch statements)?
| ui.showTaskAdded(tasks.get(index), tasks); | ||
| storage.save(tasks); | ||
| } catch (IOException e) { | ||
| ui.println("Unable to write to file"); |
There was a problem hiding this comment.
There are a lot of instances of magic numbers and strings in the codebase, please use constants instead.
| /** | ||
| * Displays the welcome message when the program starts. | ||
| */ | ||
| public void showWelcome() { |
There was a problem hiding this comment.
It would be more appropriate to use the verb "print" instead of "show".
This applies to other methods in this file.
|
|
||
| import java.util.Scanner; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Very nice usage of comments in all files.
| * | ||
| * @return A string representing the task suitable for file storage. | ||
| */ | ||
| public abstract String write(); |
There was a problem hiding this comment.
Please use more descriptive method name!
| public abstract String write(); | |
| public abstract String writeToFile(); |
| * @param keyword The keyword to search for in the task descriptions. | ||
| * @return A string containing the matching tasks with their index, or null if no match is found. | ||
| */ | ||
| public String find(String keyword) { |
There was a problem hiding this comment.
This method could use a more descriptive name as well.
change readme
No description provided.