[Li Beining] iP #453
[Li Beining] iP #453
Conversation
| package Duke.exceptions; | ||
|
|
||
| public class DukeException extends Exception { | ||
| public DukeException(String errorMessage) { |
There was a problem hiding this comment.
Can consider adding an empty line here so as to standardize with the rest of the files, just something trivial 👍
|
|
||
| import java.lang.annotation.Inherited; | ||
|
|
||
| public enum TaskType { |
There was a problem hiding this comment.
I like how you consolidate the possible task types in enum class for checking 👍
src/main/java/Duke/utils/Parser.java
Outdated
| public class Parser { | ||
|
|
||
| public static Command parse(String command) throws DukeException { | ||
| if(command.equals("bye")) { |
There was a problem hiding this comment.
Should there be a space here? Eg: if[space](command...
I notice this in some other places as well, feel free to edit! :)
| import Duke.utils.Ui; | ||
|
|
||
| public class AddCommand extends Command { | ||
| String taskType; |
There was a problem hiding this comment.
Shall we privatize variables for encapsulation? If I'm not mistaken, its part of the coding standard that this mod enforces
| Task task; | ||
| String[] parts = data.split("( \\| )"); | ||
| switch(parts[0]){ | ||
| case("T"): |
There was a problem hiding this comment.
Should there be no indentation after switch case? You can change the IDE formatting in settings for this :)
There was a problem hiding this comment.
I notice this is some other places as well. Feel free to edit
| public static void save(String path, String fileName, List<String> data) { | ||
|
|
||
| File directory = new File(path); | ||
| if (! directory.exists()){ |
There was a problem hiding this comment.
Should there be no space after "!" ?
| return tasks.size(); | ||
| } | ||
|
|
||
| public Task get(int index) { |
There was a problem hiding this comment.
could consider being more specific such as 'getTask'?
| return tasks.get(index); | ||
| } | ||
|
|
||
| public void remove(int index) { |
There was a problem hiding this comment.
Similarly could also try being more specific here?
README.md
Outdated
| @@ -15,7 +15,7 @@ Prerequisites: JDK 11, update Intellij to the most recent version. | |||
| 1. Click `Open or Import`. | |||
| 1. Select the project directory, and click `OK` | |||
| 1. If there are any further prompts, accept the defaults. | |||
| 1. After the importing is complete, locate the `src/main/java/Duke.java` file, right-click it, and choose `Run Duke.main()`. If the setup is correct, you should see something like the below: | |||
| 1. After the importing is complete, locate the `src/main/java/duke.java` file, right-click it, and choose `Run duke.main()`. If the setup is correct, you should see something like the below: | |||
There was a problem hiding this comment.
It is better for "duke" to be PascalCase.
There was a problem hiding this comment.
If I remember correctly, the standard convention is that folders will be in small letters, whereas classes to start with capital letter. Maybe you can double confirm this first before the change :)
|
|
||
| /** | ||
| * Represents command that stops the Duke from running upon execution. | ||
| */ |
| return true; | ||
| } | ||
|
|
||
| @Override |
| @@ -0,0 +1,102 @@ | |||
| package duke.task; | |||
src/main/java/duke/utils/Parser.java
Outdated
| @@ -0,0 +1,48 @@ | |||
| package duke.utils; | |||
|
|
|||
| import duke.command.*; | |||
There was a problem hiding this comment.
Can be more specific instead of using "*".
| @@ -0,0 +1,66 @@ | |||
| package duke.utils; | |||
|
|
|||
| import duke.command.*; | |||
There was a problem hiding this comment.
Can be more specific instead of using "*".
Branch a code quality
add extension help command
No description provided.