[Keith Tan] iP #429
[Keith Tan] iP #429
Conversation
In build.gradle, the dependencies on distZip and/or distTar causes
the shadowJar task to generate a second JAR file for which the
mainClass.set("seedu.duke.Duke") does not take effect.
Hence, this additional JAR file cannot be run.
For this product, there is no need to generate a second JAR file
to begin with.
Let's remove this dependency from the build.gradle to prevent the
shadowJar task from generating the extra JAR file.
src/main/java/Kif.java
Outdated
| import java.io.BufferedReader; | ||
| import java.io.IOException; | ||
| import java.io.InputStreamReader; | ||
|
|
There was a problem hiding this comment.
I see that the week 3 iP tasks were not pushed yet, but the classes should be in packages and should have header Javadoc comments on classes/methods
src/main/java/Kif.java
Outdated
| """ | ||
| ____________________________________________________________ | ||
| Hello! I'm Kif | ||
| What can I do for you? | ||
| ____________________________________________________________ | ||
| """; |
There was a problem hiding this comment.
Huh, I wasn't aware of text blocks in Java. Good idea to use them!
Though I'm not sure of the code style requirements for their indentation...
src/main/java/Kif.java
Outdated
| introduce(); | ||
| String userMessage = reader.readLine(); | ||
| String[] splitMessage = userMessage.split(" "); | ||
| while(!isTerminate(splitMessage[0])) { |
There was a problem hiding this comment.
Shouldn't there be a whitespace after while here?
Noticed similar occurences in the rest of the code as well.
src/main/java/Kif.java
Outdated
| switch (command) { | ||
| case LIST -> Task.listUserTask(); | ||
| case MARK -> Task.markUserTask(Integer.parseInt(splitMessage[1])); | ||
| case UNMARK -> Task.unmarkUserTask(Integer.parseInt(splitMessage[1])); | ||
| case DEADLINE -> Task.Deadline.createDeadline(userMessage); | ||
| case EVENT -> Task.Event.createEvent(userMessage); | ||
| case DELETE -> Task.delete(Integer.parseInt(splitMessage[1])); | ||
| case TODO -> { | ||
| try { | ||
| Task.ToDo.createToDo(userMessage); | ||
| } catch (KifException e) { | ||
| System.out.println(e.getMessage()); | ||
| } | ||
| } | ||
| default -> System.out.println( | ||
| """ | ||
| ____________________________________________________________ | ||
| OOPS!!! I'm sorry, but I don't know what that means :-( | ||
| ____________________________________________________________""" | ||
| ); |
There was a problem hiding this comment.
The case statements shouldn't be indented according to the coding standard?
src/main/java/Task.java
Outdated
| private static final ArrayList<Task> userTasks = new ArrayList<>(); | ||
| protected String description; | ||
| protected boolean isDone; | ||
| private static final String FILEPATH = "tasks.txt"; | ||
| protected taskType type; | ||
| private static final String KEYWORD = "kifReservedKeyword"; |
There was a problem hiding this comment.
Would grouping variables by their access modifiers be better? For instance, grouping the private static final variables together etc...
src/main/java/Task.java
Outdated
| } | ||
| } | ||
| } catch (FileNotFoundException e) { | ||
| //do nothing |
There was a problem hiding this comment.
I would recommend handling the exception. This could be in the form of printing an error message to the user to inform them that there was an issue with the ongoing process
src/main/java/Task.java
Outdated
| private static void editTaskTXT (int lineNumber, Kif.UserCommand operation) { | ||
| ArrayList<String> preItems = new ArrayList<>(); | ||
| ArrayList<String> postItems = new ArrayList<>(); | ||
| ArrayList<String> editLine = new ArrayList<>(); |
There was a problem hiding this comment.
Would changing the variable name to a plural form be more inline with the Java Coding Standard for collection of objects?
src/main/java/Task.java
Outdated
| } | ||
|
|
||
| public static void listUserTask() { | ||
| System.out.println("____________________________________________________________"); |
There was a problem hiding this comment.
Would extracting the line into a static final variable provide a cleaner body for the methods? I noticed this line is used quite extensively in this file
src/main/java/Task.java
Outdated
| public static void markUserTask(int key) { | ||
| Task currentTask = userTasks.get(key - 1); | ||
| currentTask.isDone = true; | ||
| editTaskTXT(key, Kif.UserCommand.MARK); |
There was a problem hiding this comment.
| editTaskTXT(key, Kif.UserCommand.MARK); | |
| editTaskTxt(key, Kif.UserCommand.MARK); |
There was a problem hiding this comment.
Perhaps this would be more in line with the java coding standard
src/main/java/Kif.java
Outdated
| public enum UserCommand { | ||
| LIST, | ||
| MARK, | ||
| UNMARK, | ||
| DEADLINE, | ||
| EVENT, | ||
| TODO, | ||
| DELETE; | ||
| } |
There was a problem hiding this comment.
I love that you created an enum for the types of commands!!
No description provided.