【Heng Jee Kuan] iP #457
【Heng Jee Kuan] iP #457
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.
| ui.printTaskList(tasks.getTasks()); | ||
| } else if (input.startsWith("todo")) { | ||
| String[] parts = input.split(" ", 2); | ||
| if (parts.length < 2) throw new JeeniusException("bro how do you todo nothing??? ADD A DESCRIPTION FOR YOUR TODO"); |
There was a problem hiding this comment.
The conditional should be put on a separate line, could help in future debugging if necessary :>!
Other than that LGTM!!
| boolean isDone = parts[1].trim().equals("1"); | ||
| String description = parts[2].trim(); | ||
|
|
||
| switch (taskType) { |
There was a problem hiding this comment.
The case clauses shouldn't have any indentation as per coding standards. Can try to configure IDE to standardise the style
| return this.description; | ||
| } | ||
|
|
||
| public Boolean getDone() { |
There was a problem hiding this comment.
This getter function can follow the "use a prefix such as is, has, was, etc. for boolean variables and methods". (Small things man give chance need fill quota)
| loadedTasks = new TaskList(storage.load()); | ||
| } catch (JeeniusException e) { | ||
| ui.printError(e.getMessage()); | ||
| loadedTasks = new TaskList(new ArrayList<>()); |
There was a problem hiding this comment.
The Generic here could be filled with the super class (Task) for clarity, readability, and type safety. Prevent any inference made by Java.
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class TaskList { |
| import java.time.LocalDateTime; | ||
| import java.time.format.DateTimeFormatter; | ||
|
|
||
| public class Deadline extends Task { |
| import java.time.LocalDateTime; | ||
| import java.time.format.DateTimeFormatter; | ||
|
|
||
| public class Event extends Task { |
|
|
||
| import java.util.List; | ||
|
|
||
| public class Ui { |
There was a problem hiding this comment.
Good job man, there's really nothing to nitpick at all
| ui.printError(e.getMessage()); | ||
| } | ||
| } | ||
| } |
| ui.printTaskList(tasks.getTasks()); | ||
| } else if (input.startsWith("todo")) { | ||
| String[] parts = input.split(" ", 2); | ||
| if (parts.length < 2) throw new JeeniusException("bro how do you todo nothing??? ADD A DESCRIPTION FOR YOUR TODO"); |
There was a problem hiding this comment.
Line is too long, can be difficult to read. consider splitting into 2 lines.
| } catch (Exception e) { | ||
| throw new JeeniusException("Failed to mark/unmark. Use: mark/unmark [task number]"); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
I like the way you format the code in each "if" case. Very readable and neat, except for the few lines that seem to be very long and can be split into 2.
| public Task getTask(int index) { | ||
| return tasks.get(index); | ||
| } | ||
|
|
There was a problem hiding this comment.
Should name method getSize to follow naming standard for methods. Apart from that LGTM
|
|
||
| public class Ui { | ||
|
|
||
| public void printWelcomeMessage() { |
There was a problem hiding this comment.
Good use of already defined functions to neaten and reduce repeated code. LGTM
No description provided.