[Hsien Kit] iP#102
Conversation
Created a new class called Task to hold the description and whether the task is done or not V can now parse commands and check the first word to see what kind of command to execute
| private final String TYPE = "D"; | ||
| private String by; |
There was a problem hiding this comment.
Consider declaring attributes as protected rather than private as it would be easier to facilitate extension of subclasses as the project progress.
| String logo = " _ _ \n" | ||
| + "\\ \\ / / \n" | ||
| + " \\ \\ / / \n" | ||
| + " \\ \\_/ / \n" | ||
| + " \\___/ \n"; |
There was a problem hiding this comment.
Consider to declare as public static final
| boolean isOnline = true; | ||
| Task[] listOfTasks = new Task[100]; | ||
| int count = 0; | ||
| String temp; |
There was a problem hiding this comment.
variable name "temp" can be more descriptive.
| case "bye": | ||
| input.close(); | ||
| isOnline = false; | ||
| break; |
There was a problem hiding this comment.
Good use of boolean flag isOnline as it allows the program to have a clear flow and makes the code easy to understand.
shaunngoh
left a comment
shaunngoh
left a comment
There was a problem hiding this comment.
Overall, I found that your code was easy to read, though maintainability might be slightly harder due to the complex nature of your main function. Goodjob!
| public static void main(String[] args) { | ||
| greet(); | ||
|
|
||
| boolean isOnline = true; | ||
| Task[] listOfTasks = new Task[100]; | ||
| int count = 0; | ||
| String temp; | ||
| String description; | ||
| String by; | ||
| String[] timePeriod; | ||
| String from; | ||
| String to; | ||
| String line; | ||
| String[] lineArr; | ||
| Scanner input = new Scanner(System.in); | ||
|
|
||
| while (isOnline) { | ||
| line = input.nextLine(); | ||
| lineArr = line.trim().split(" "); | ||
| switch (lineArr[0]) { | ||
| case "bye": | ||
| input.close(); | ||
| isOnline = false; | ||
| break; | ||
| case "list": | ||
| displayList(listOfTasks, count); | ||
| break; | ||
| case "mark": | ||
| int position = Integer.parseInt(lineArr[1]); | ||
| listOfTasks[position - 1].setDone(); | ||
| displayList(listOfTasks, count); | ||
| break; | ||
| case "todo": | ||
| description = String.join(" ", Arrays.copyOfRange(lineArr, 1, lineArr.length)); | ||
| listOfTasks[count] = new ToDo(description); | ||
| count++; | ||
| break; | ||
| case "event": | ||
| temp = String.join(" ", Arrays.copyOfRange(lineArr, 1, lineArr.length)); | ||
| description = temp.split("/from")[0]; | ||
| timePeriod = temp.split("/from")[1].split("/to"); | ||
| from = timePeriod[0]; | ||
| to = timePeriod[1]; | ||
| listOfTasks[count] = new Event(description, from, to); | ||
| System.out.println(listOfTasks[count]); | ||
| count++; | ||
| break; | ||
| case "deadline": | ||
| temp = String.join(" ", Arrays.copyOfRange(lineArr, 1, lineArr.length)); | ||
| description = temp.split("/by")[0]; | ||
| by = temp.split("/by")[1]; | ||
| listOfTasks[count] = new Deadline(description, by); | ||
| System.out.println(listOfTasks[count]); | ||
| count++; | ||
| break; | ||
| default: | ||
| System.out.println("Try again"); | ||
| break; | ||
| } | ||
| } | ||
| printBlock("See Ya"); | ||
| } |
There was a problem hiding this comment.
You could consider shortening your main method by breaking down the respective tasks into smaller methods
| @@ -0,0 +1,25 @@ | |||
| public class Event extends Task { | |||
| private final String TYPE = "E"; | |||
There was a problem hiding this comment.
Good use of declaring a constant to determine the type of task when returning from the getType method!
| boolean isOnline = true; | ||
| Task[] listOfTasks = new Task[100]; | ||
| int count = 0; | ||
| String temp; | ||
| String description; | ||
| String by; | ||
| String[] timePeriod; | ||
| String from; | ||
| String to; | ||
| String line; | ||
| String[] lineArr; | ||
| Scanner input = new Scanner(System.in); |
There was a problem hiding this comment.
The respective variables here are hard to follow, maybe you could provide a comment for each of the variable's use case?
| String[] lineArr; | ||
| Scanner input = new Scanner(System.in); | ||
|
|
||
| while (isOnline) { |
There was a problem hiding this comment.
Good naming convention for your boolean variable!
A NumberFormatException is thrown when the user does not input a valid integer when marking a task A InvalidDeadlineException is thrown when the user does not add a "/by" or a deadline after the "/by"
That comment line was meant for master branch, not branch-Level-7
Initially the file name was Duke and the programme could not be compiled
Created a new class called TaskList to handle list functions
V will no longer display an empty list if the save file was empty
V was printing to terminal when loading the save file. This bug was fixed by adding a boolean isFromSaveFile parameter to the add tasks function where if the task was added from the save file, the function will not print the output of the addition of the task
Branch level 9
Merge branch-A-JavaDoc to master
okkhoy
left a comment
okkhoy
left a comment
There was a problem hiding this comment.
In general, code is well written. Try to organize the code as per A-Packages for better code management. Pay attention to (some of the) method names.
| wordArray = command.trim().split(" "); | ||
| switch (wordArray[0].toLowerCase()) { |
| taskList.deleteTask(position); | ||
| return true; | ||
| case "todo": | ||
| description = String.join(" ", Arrays.copyOfRange(wordArray, 1, wordArray.length)); |
There was a problem hiding this comment.
can SLAP this out; see the example from previous lecture.
| * Loads save file using file path saved in <code>saveFilePath</code> attribute. | ||
| * Stores all the commands as a String in an array. | ||
| */ | ||
| public void loadSave() { |
There was a problem hiding this comment.
the name can be better. reason: does loadSave mean load (read from file) or save (write to file)?
| * A new file is created if it a save file does not exist. | ||
| * @param taskList list of tasks | ||
| */ | ||
| public void createSave(TaskList taskList) { |
| for (Task task: list) { | ||
| switch(task.getType()) { | ||
| case "T": | ||
| saveFileWriter.write("todo " + task.getDescription() + System.lineSeparator()); |
There was a problem hiding this comment.
you do have a toString() method, right? can try to us that here.
| } | ||
|
|
||
| /** | ||
| * @return ArrayList of String containing all the commands from save file |
There was a problem hiding this comment.
better: Returns the ArrayList of string ...
4 participants
No description provided.