[ChinTaiAnn] ip#76
Conversation
suenalaba
left a comment
There was a problem hiding this comment.
You can try refactoring your code, to make it for readable. Avoid too many commands within a block.
| System.out.println(("What can i do for you?")); | ||
| System.out.println("---------------------"); | ||
| Task[] taskList = new Task[100]; | ||
| Boolean run = true; |
There was a problem hiding this comment.
Naming error. Boolean not does not sound boolean
| else if (anyString.matches("mark \\d+")){ | ||
| String[] arrOfStr = anyString.split(" ", 0); | ||
| String indexValue = arrOfStr[1]; | ||
| int indexValue2 = Integer.parseInt(indexValue) - 1; | ||
| taskList[indexValue2].markTask(); | ||
|
|
||
| } | ||
|
|
||
| else if (anyString.startsWith("todo")) { | ||
| String desc = anyString.substring(5); | ||
| taskList[arrayIndex] = new Task(desc); | ||
| arrayIndex++; | ||
| } | ||
|
|
||
| else if (anyString.startsWith("deadline")){ | ||
| int index = anyString.indexOf("/"); | ||
| String desc = anyString.substring(9,index); | ||
| index++; | ||
| String by = anyString.substring(index); | ||
| taskList[arrayIndex] = new Deadline(desc,by); | ||
| arrayIndex++; |
There was a problem hiding this comment.
Can refactor code to make it more readable.
|
|
||
| @Override | ||
| public String toString() { | ||
| return "[E]" + (super.isDone ? "[X]" + super.description : "[ ]"+ super.description) + "(" + timing + ")" ; // mark done task with X |
There was a problem hiding this comment.
Not readable code. Can try refactoring.
| int arrayIndex = 0; | ||
| while (run) { | ||
| Scanner sc = new Scanner(System.in); | ||
| String anyString = sc.nextLine(); |
There was a problem hiding this comment.
Inappropriate naming. Should name sc to something more appropriate
| public String toString() { | ||
| return "[T]" + (isDone ? "[X]"+ this.description : "[ ]"+ this.description); // mark done task with X | ||
| } | ||
|
|
There was a problem hiding this comment.
Should comment above similar to an indentation.
thernturnaround
left a comment
There was a problem hiding this comment.
Code looks mostly fine, but adding javadocs would make the code more readable and remove superfluous comments.
| + "| |_| | |_| | < __/\n" | ||
| + "|____/ \\__,_|_|\\_\\___|\n"; | ||
| System.out.println("Hello from\n" + logo); | ||
| System.out.println("---------------------"); |
| } | ||
|
|
||
| public String toString() { | ||
| return "[T]" + (isDone ? "[X]"+ this.description : "[ ]"+ this.description); // mark done task with X |
There was a problem hiding this comment.
Could add new lines to improve readability, and comment could be incorporated as part of javadocs
| return "[T]" + (isDone ? "[X]"+ this.description : "[ ]"+ this.description); // mark done task with X | |
| /* | |
| * @return (what it returns) | |
| */ | |
| return "[T]" | |
| + ( (isDone ? "[X]"+ this.description) : "[ ]"+ this.description); |
| arrayIndex++; | ||
| } | ||
|
|
||
| else if (anyString.startsWith("deadline")){ |
There was a problem hiding this comment.
Whitespace consistency
| else if (anyString.startsWith("deadline")){ | |
| else if (anyString.startsWith("deadline")) { |
| arrayIndex++; | ||
| } | ||
|
|
||
| else if (anyString.startsWith("event")){ |
There was a problem hiding this comment.
Whitespace consistency
| else if (anyString.startsWith("event")){ | |
| else if (anyString.startsWith("event")) { |
| System.out.println("Bye! Hope to see you again."); | ||
| run = false; | ||
| } | ||
| else if (anyString.matches("mark \\d+")){ |
There was a problem hiding this comment.
Whitespace consistency
| else if (anyString.matches("mark \\d+")){ | |
| else if (anyString.matches("mark \\d+")) { |
|
|
||
| @Override | ||
| public String toString() { | ||
| return "[E]" + (super.isDone ? "[X]" + super.description : "[ ]"+ super.description) + "(" + timing + ")" ; // mark done task with X |
There was a problem hiding this comment.
Could add new lines to improve readability, and comment could be incorporated as part of javadocs
| return "[E]" + (super.isDone ? "[X]" + super.description : "[ ]"+ super.description) + "(" + timing + ")" ; // mark done task with X | |
| /* | |
| * @return (what it returns) | |
| */ | |
| return "[E]" | |
| + (super.isDone ? "[X]" + super.description : "[ ]"+ super.description) | |
| + "(" + timing + ")" ; |
| @Override | ||
| public String toString() { | ||
| // return "[D]" + super.toString() + "(" + by + ")"; | ||
| return "[D]" + (super.isDone ? "[X]" + super.description : "[ ]"+ super.description) + "(" + by + ")" ; // mark done task with X |
There was a problem hiding this comment.
Could add new lines to improve readability, and comment could be incorporated as part of javadocs.
Could also clean up line 13
| return "[D]" + (super.isDone ? "[X]" + super.description : "[ ]"+ super.description) + "(" + by + ")" ; // mark done task with X | |
| /* | |
| * @return (what it returns) | |
| */ | |
| return "[D]" | |
| + (super.isDone ? "[X]" + super.description : "[ ]"+ super.description) | |
| + "(" + by + ")" ; |
xseh
left a comment
There was a problem hiding this comment.
Take note of the coding standards, naming convention and improve readability by refactoring and avoiding magic literals.
| public class Duke { | ||
| public static void main(String[] args) throws IndexOutOfBoundsException{ | ||
| System.out.println("---------------------"); | ||
| System.out.println(("Hello! I'm Duke")); | ||
| System.out.println(("What can i do for you?")); | ||
| System.out.println("---------------------"); | ||
|
|
||
| ArrayList<Task> taskList = new ArrayList<Task>(); | ||
| //file organization | ||
| try { | ||
| File myTasks = new File("tasks.txt"); | ||
| if (myTasks.createNewFile()){ | ||
| System.out.println("File is created!"); | ||
| }else{ | ||
| System.out.println("File exists, reading it to taskList!"); | ||
| Scanner s = new Scanner(myTasks); // create a Scanner using the File as the source | ||
| while (s.hasNext()){ | ||
| String nextString = s.nextLine(); | ||
| String[] splitString = nextString.split("\\|"); | ||
| String typeTask = splitString[0].replaceAll(" ",""); | ||
| String marked = splitString[1].replaceAll(" ",""); | ||
| String desc = splitString[2]; | ||
| System.out.println(typeTask); | ||
| System.out.println(marked); | ||
| System.out.println(desc); | ||
| String todo = "T"; | ||
| String deadline = "D"; | ||
| String event = "E"; | ||
| String toMark = "1"; | ||
| if (typeTask.equals(todo)) { | ||
| taskList.add(new Todo(desc)); | ||
| if (marked.equals(toMark)) { | ||
| taskList.get(taskList.size()-1).markTask(); | ||
| } | ||
| } | ||
|
|
||
| else if (typeTask.equals(deadline)) { | ||
| String deadlineDate = splitString[3]; | ||
| taskList.add(new Deadline(desc,deadlineDate)); | ||
| if (marked.equals(toMark)) { | ||
| taskList.get(taskList.size()-1).markTask(); | ||
| } | ||
| } | ||
|
|
||
| else if (typeTask.equals(event)) { | ||
| String eventDate = splitString[3]; | ||
| taskList.add(new Event(desc,eventDate)); | ||
| if (marked.equals(toMark)) { | ||
| taskList.get(taskList.size()-1).markTask(); | ||
| } | ||
| } | ||
| } | ||
| s.close(); | ||
| } | ||
|
|
||
| } catch (IOException e) { | ||
| e.printStackTrace(); | ||
| } | ||
|
|
||
| String filePath = "tasks.txt"; | ||
| boolean continueProgram = true; | ||
|
|
||
| //using Arraylist instead. | ||
| boolean continueProgram = true; | ||
|
|
||
| DukeException dukeEx = new DukeException(); | ||
| while (continueProgram) { | ||
| Scanner sc = new Scanner(System.in); | ||
| String inputString = sc.nextLine(); | ||
| if (inputString.equals("bye")) { | ||
| System.out.println("Bye! Hope to see you again."); | ||
| continueProgram = false; | ||
| } | ||
|
|
||
|
|
||
| //deleting task | ||
| else if (inputString.matches("delete \\d+")){ | ||
| try { | ||
| String[] arrOfStr = inputString.split(" ", 0); | ||
| String indexValue = arrOfStr[1]; | ||
| int indexValue2 = Integer.parseInt(indexValue) - 1; | ||
| if (indexValue2 > taskList.size()-1 || indexValue2 < 0){ | ||
| throw new IndexOutOfBoundsException(); | ||
| } | ||
|
|
||
| Task removedTask = taskList.get(indexValue2); | ||
| taskList.remove(indexValue2); | ||
| System.out.println("Noted. I've removed this task:"); | ||
| System.out.println(removedTask.toString()); | ||
| System.out.println("Now you have " + taskList.size() + " tasks in the list."); | ||
|
|
||
| updateFile(filePath,taskList); | ||
|
|
||
|
|
||
| } | ||
|
|
||
| catch (IndexOutOfBoundsException e){ | ||
| dukeEx.wrongIndex(); | ||
| } | ||
| } | ||
|
|
||
| //marking a task by index | ||
| else if (inputString.matches("mark \\d+")){ | ||
|
|
||
| try { | ||
| String[] arrOfStr = inputString.split(" ", 0); | ||
| String indexValue = arrOfStr[1]; | ||
| int indexValue2 = Integer.parseInt(indexValue) - 1; | ||
| //index given is out of array size | ||
| if (indexValue2 > taskList.size()-1 || indexValue2 < 0){ | ||
| throw new IndexOutOfBoundsException(); | ||
| } | ||
|
|
||
|
|
||
| taskList.get(indexValue2).markTask(); | ||
| updateFile(filePath,taskList); | ||
| } | ||
|
|
||
| catch (IndexOutOfBoundsException e){ | ||
| dukeEx.wrongIndex(); | ||
| } | ||
| } | ||
|
|
||
| //unmarking a task by index | ||
| else if (inputString.matches("unmark \\d+")){ | ||
|
|
||
| try { | ||
| String[] arrOfStr = inputString.split(" ", 0); | ||
| String indexValue = arrOfStr[1]; | ||
| int indexValue2 = Integer.parseInt(indexValue) - 1; | ||
| if (indexValue2 > taskList.size()-1 || indexValue2 < 0){ | ||
| throw new IndexOutOfBoundsException(); | ||
| } | ||
| taskList.get(indexValue2).unmarkTask(); | ||
| updateFile(filePath,taskList); | ||
|
|
||
| } | ||
|
|
||
| catch (IndexOutOfBoundsException e){ | ||
| dukeEx.wrongIndex(); | ||
| } | ||
| } | ||
|
|
||
| else if (inputString.startsWith("todo")) { | ||
|
|
||
| try { | ||
| String desc = inputString.substring(5); | ||
| if (desc.isEmpty() || desc.isBlank()) { | ||
| throw new IndexOutOfBoundsException(); | ||
| } | ||
| System.out.println("Got it. I've added this task:"); | ||
|
|
||
|
|
||
| taskList.add(new Todo(desc)); | ||
| System.out.println(taskList.get(taskList.size()-1).toString()); | ||
| System.out.println("Now you have " + taskList.size() + " tasks in the list."); | ||
| String textToAdd = "T" + "|" + " 0 " + "|" + desc; | ||
| try { | ||
| appendData(filePath,textToAdd); | ||
| } | ||
| catch (IOException e) { | ||
| System.out.println("Failed to write event data to text file."); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| catch (IndexOutOfBoundsException e){ | ||
| dukeEx.missingDescription("todo"); | ||
| } | ||
| } | ||
|
|
||
| else if (inputString.startsWith("deadline")){ | ||
|
|
||
| try { | ||
|
|
||
| int index = inputString.indexOf("/"); | ||
| String desc = inputString.substring(9,index); | ||
| if (desc.isEmpty() || desc.isBlank()) { | ||
| throw new IndexOutOfBoundsException(); | ||
| } | ||
| index++; | ||
| String by = inputString.substring(index); | ||
| if (by.isEmpty() || by.isBlank()) { | ||
| throw new IndexOutOfBoundsException(); | ||
| } | ||
| // taskList[arrayIndex] = new Deadline(desc,by); | ||
| taskList.add(new Deadline(desc,by)); | ||
| System.out.println("Got it. I've added this task:"); | ||
|
|
||
| System.out.println(taskList.get(taskList.size()-1).toString()); | ||
| System.out.println("Now you have " + taskList.size() + " tasks in the list."); | ||
| String textToAdd = "D" + "|" + " 0 " + "|" + desc + "|" + by; | ||
| try { | ||
| appendData(filePath,textToAdd); | ||
| } | ||
| catch (IOException e) { | ||
| System.out.println("Failed to write deadline data to text file."); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| catch (IndexOutOfBoundsException e){ | ||
| dukeEx.missingDescription("deadline"); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| else if (inputString.startsWith("event")){ | ||
|
|
||
| try { | ||
|
|
||
| int index = inputString.indexOf("/"); | ||
| String desc = inputString.substring(6,index); | ||
| if (desc.isEmpty() || desc.isBlank()) { | ||
| throw new IndexOutOfBoundsException(); | ||
| } | ||
| index++; | ||
| String by = inputString.substring(index); | ||
| if (by.isEmpty() || by.isBlank()) { | ||
| throw new IndexOutOfBoundsException(); | ||
| } | ||
| // taskList[arrayIndex] = new Event(desc,by); | ||
| taskList.add(new Event(desc,by)); | ||
| System.out.println("Got it. I've added this task:"); | ||
| System.out.println(taskList.get(taskList.size()-1).toString()); | ||
| System.out.println("Now you have " + taskList.size() + " tasks in the list."); | ||
|
|
||
| String textToAdd = "E" + "|" + " 0 " + "|" + desc + "|" + by; | ||
| try { | ||
| appendData(filePath,textToAdd); | ||
| } | ||
| catch (IOException e) { | ||
| System.out.println("Failed to write event data to text file."); | ||
| } | ||
| } | ||
|
|
||
| catch (IndexOutOfBoundsException e){ | ||
| dukeEx.missingDescription("event"); | ||
| } | ||
|
|
||
|
|
||
|
|
||
|
|
||
| } | ||
|
|
||
| else if (inputString.equals("list")){ | ||
| System.out.println("Here are the tasks in your list : "); | ||
| for (int i = 0; i<taskList.size(); i++) { | ||
| System.out.println(i+1 + "." + taskList.get(i).toString()); | ||
| } | ||
| } | ||
|
|
||
| else { | ||
| System.out.println("☹ OOPS!!! I'm sorry, but I don't know what that means :-("); | ||
| continue; | ||
| } | ||
|
|
||
|
|
||
|
|
||
|
|
||
| } | ||
|
|
||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
Avoid long methods (> 30 LOCs) to improve code readability.
There was a problem hiding this comment.
Avoid deep nesting (> 3 levels of indentation) in the code to improve readability.
| System.out.println("---------------------"); | ||
| System.out.println(("Hello! I'm Duke")); | ||
| System.out.println(("What can i do for you?")); | ||
| System.out.println("---------------------"); |
There was a problem hiding this comment.
Avoid magic literals for improved readability. For example, the string "---------------------" can be stored in the variable MESSAGE_BORDER.
| File myTasks = new File("tasks.txt"); | ||
| if (myTasks.createNewFile()){ | ||
| System.out.println("File is created!"); | ||
| }else{ |
There was a problem hiding this comment.
According to the Java coding standard, there should be blank spaces between }else{.
| } | ||
| } | ||
|
|
||
| else if (typeTask.equals(deadline)) { |
There was a problem hiding this comment.
According to the Java coding standard, if-else statement should be in the form:
if (condition) {
statements;
} else if (condition) {
statements;
} else {
statements;
}
| } | ||
|
|
||
| else if (typeTask.equals(deadline)) { | ||
| String deadlineDate = splitString[3]; |
There was a problem hiding this comment.
Avoid magic indexes for better readability. For example, splitString[3] can be changed to splitString[DATE_INDEX].
| } | ||
|
|
||
|
|
||
| taskList.get(indexValue2).markTask(); |
There was a problem hiding this comment.
Consider using a more descriptive name to improve code readability.
| if (by.isEmpty() || by.isBlank()) { | ||
| throw new IndexOutOfBoundsException(); | ||
| } | ||
| // taskList[arrayIndex] = new Deadline(desc,by); |
| throw new IndexOutOfBoundsException(); | ||
| } | ||
| // taskList[arrayIndex] = new Deadline(desc,by); | ||
| taskList.add(new Deadline(desc,by)); |
There was a problem hiding this comment.
According to the Java coding standards, there should be a white space between the variables desc,by
|
|
||
| else if (inputString.equals("list")){ | ||
| System.out.println("Here are the tasks in your list : "); | ||
| for (int i = 0; i<taskList.size(); i++) { |
There was a problem hiding this comment.
According to the Java coding standards, the loop structure should resemble this (take note of the white spaces)
for (i = 0; i < 100; i++) {
sum += value[i];
}
| throw new IndexOutOfBoundsException(); | ||
| } | ||
| taskList.get(indexValue2).unmarkTask(); | ||
| updateFile(filePath,taskList); |
There was a problem hiding this comment.
According to the Java coding standards,
plural form should be used on names representing a collection of objects
No description provided.