iP #187
iP #187
Conversation
src/main/java/Duke.java
Outdated
| System.out.println("\nNow you have " + taskCount + " tasks in the list.\n" + line); | ||
| } | ||
|
|
||
| //Filters user inputs and pushes to different methods |
There was a problem hiding this comment.
No coding violations found here, I like the use of comments at the top to give a brief explanation of what the function does
src/main/java/Duke.java
Outdated
| public static void sayList() { | ||
| System.out.println(line); | ||
| for (int i = 0; i < taskCount; i++) { | ||
| System.out.println((i + 1) + ". " + t[i].toString()+ "\n"); |
There was a problem hiding this comment.
the + between t[i].toString() and "\n" needs to have a spacing in between
src/main/java/Duke.java
Outdated
| public static String line = "------------------------------------------------------------------------------------------\n"; | ||
|
|
||
| //declare task array and keep track of how many tasks stored | ||
| public static Task[] t = new Task[100]; |
There was a problem hiding this comment.
No coding violations found, but maybe you can declare 100 as a constant to indicate that it is the maximum size of the list.
src/main/java/Duke.java
Outdated
| int endIndex = input.lastIndexOf("/"); | ||
| String taskName = input.substring(9, endIndex); | ||
| int endIndex2 = input.length(); | ||
| String by = input.substring(endIndex + 4, endIndex2); |
There was a problem hiding this comment.
No coding violation found, but I feel like it would be good if you declared the 4 here, 9 above in taskName and 5 above in the previous taskName as constants to make it apparent that these are offsets
src/main/java/Duke.java
Outdated
| //Adds a new todo task and prints it //todo borrow book | ||
| public static void sayTodo(String input) { | ||
| int endIndex = input.length(); | ||
| String taskName = input.substring(5, endIndex); |
There was a problem hiding this comment.
magic int. should use a constant instead of "5".
src/main/java/Duke.java
Outdated
| } | ||
|
|
||
| //Adds a new deadline task and prints it | ||
| public static void sayDeadline(String input) { //deadline return book /by Sunday |
There was a problem hiding this comment.
This method can be split into a few different methods. "sayDeadline" seems to imply printing like in sayBye() but also performs the adding of a new deadline object
src/main/java/Duke.java
Outdated
| public static void sayEvent(String input) { //event project meeting /at Mon 2-4pm | ||
| int endIndex = input.lastIndexOf("/"); | ||
| String taskName = input.substring(6, endIndex); | ||
| int endIndex2 = input.length(); |
There was a problem hiding this comment.
Name endIndex and endIndex2 differently to explain what the variables are/do
# Conflicts: # src/main/java/duke/Duke.java
There was a problem hiding this comment.
Great work Pragyan! I can tell that you have definitely followed various coding guidelines and your Duke is so awesome! 💯
Overall, this version of code is very easy to understand and very readable. Please take some time to fix some very few potential and minor coding standard and quality-related problems and then it should be good to go. Keep up the good work :)
| @@ -0,0 +1,25 @@ | |||
| package duke; | |||
There was a problem hiding this comment.
Great to see that you organise your types (e.g., classes) into a package for easier management!
src/main/java/duke/Duke.java
Outdated
| //declarations | ||
| public static String line = "------------------------------------------------------------------------------------------\n"; | ||
| private static int quitFlag = 0; //if 0, take user input. Otherwise, don't. |
There was a problem hiding this comment.
Lines 11 and 12 are quite obvious and self-explanatory, perhaps you could refrain from repeating the description in a comment.
src/main/java/duke/Duke.java
Outdated
| import java.nio.file.StandardOpenOption; | ||
| import java.util.Scanner; | ||
| import java.util.ArrayList; | ||
| import java.io.*; |
There was a problem hiding this comment.
Perhaps you could avoid using a wildcard in the import statement by configuring your IDE to make all imported classes to be listed explicitly.
src/main/java/duke/Duke.java
Outdated
|
|
||
| public class Duke { | ||
| //declarations | ||
| public static String line = "------------------------------------------------------------------------------------------\n"; |
There was a problem hiding this comment.
Perhaps you could make line final if it does not change, and change the naming accordingly (i.e., LINE).
src/main/java/duke/Duke.java
Outdated
|
|
||
| //Saves Task list into local file | ||
| public static void saveData(ArrayList<Task> t) { | ||
| String path = "D:\\Documents\\NUS\\Y2S1\\CS2113T\\IP\\UserData.txt"; |
There was a problem hiding this comment.
Perhaps you could use a relative path here rather than an absolute path because your app might cause unpredictable results when used in another computer.
src/main/java/duke/Duke.java
Outdated
| } | ||
|
|
||
| //Program exits with this ending | ||
| public static void sayBye(String input) throws DukeException{ //bye |
There was a problem hiding this comment.
Perhaps you could leave a space here before { for layout consistency.
src/main/java/duke/Duke.java
Outdated
| //Loads Task list into Duke | ||
| public static void loadData() throws FileNotFoundException, DukeException { | ||
| File f = new File("D:\\Documents\\NUS\\Y2S1\\CS2113T\\IP\\UserData.txt"); | ||
|
|
||
| Scanner scan = new Scanner(f); | ||
| String taskType; | ||
| String s; | ||
| int taskNumber = 1; //tracks how many tasks read from .txt file so far | ||
|
|
||
| while(scan.hasNext()) //todo hello | 1 | ||
| { //deadline hello /by Sunday | 1 | ||
| String data = scan.nextLine(); //event project meeting /at Mon 2-4pm | 0 | ||
| String[] arrayString = data.split(" \\| "); | ||
| String[] arrayString2 = arrayString[0].split(" "); | ||
| taskType = arrayString2[0]; | ||
| s = Integer.toString(taskNumber); | ||
|
|
||
| switch (taskType) { | ||
| case "todo": | ||
| sayTodo(arrayString[0]); | ||
| if (arrayString[1].equals("1")) { | ||
| sayDone(s); | ||
| } | ||
| break; | ||
| case "deadline": | ||
| sayDeadline(arrayString[0]); | ||
| if (arrayString[1].equals("1")) { | ||
| sayDone(s); | ||
| } | ||
| break; | ||
| case "event": | ||
| sayEvent(arrayString[0]); | ||
| if (arrayString[1].equals("1")) { | ||
| sayDone(s); | ||
| } | ||
| break; | ||
| default: | ||
| } | ||
| taskNumber++; | ||
| } | ||
| scan.close(); | ||
| } |
There was a problem hiding this comment.
This method exceeds 30 LOC, perhaps you could shorten it (e.g., by extracting lines 64 to lines 84 into a method on its own).
src/main/java/duke/Duke.java
Outdated
| saveData(t); | ||
| break; | ||
| case "todo": | ||
| sayTodo(input); | ||
| saveData(t); | ||
| break; | ||
| case "deadline": | ||
| sayDeadline(input); | ||
| saveData(t); | ||
| break; | ||
| case "event": | ||
| sayEvent(input); | ||
| saveData(t); | ||
| break; | ||
| case "delete": //lvl 7 | ||
| sayDelete(input); | ||
| saveData(t); |
There was a problem hiding this comment.
Looks like the "saveData(t)" is repeated mutiple times under different case statements, you could consider calling it only once to avoid repetitive code.
src/main/java/duke/Duke.java
Outdated
| //Main | ||
| public static void main(String[] args) throws DukeException, IOException { | ||
| start(); | ||
| inputSort(); | ||
| } | ||
| } |
There was a problem hiding this comment.
All statements here are at the same level of abstraction, great! You have followed of Single Level of Abstraction Principle (SLAP) very well!
src/main/java/duke/Duke.java
Outdated
| while(scan.hasNext()) //todo hello | 1 | ||
| { //deadline hello /by Sunday | 1 | ||
| String data = scan.nextLine(); //event project meeting /at Mon 2-4pm | 0 |
There was a problem hiding this comment.
Perhaps you could rewrite your while loop as per https://se-education.org/guides/conventions/java/index.html#layout so that { appear as the same line as while().
Add Level-8
# Conflicts: # src/main/java/duke/Duke.java
# Conflicts: # src/main/java/duke/Duke.java
Branch level 9
# Conflicts: # UserData.txt
Added levels 0-3 and A-CodingStandard