[Toh Hai Jie Joey] iP #343
Conversation
Remove wildcard imports and declare constants as static final fields
src/main/java/Event.java
Outdated
| } | ||
|
|
||
| /** | ||
| * An overridden method that returns a String with the Task type, |
There was a problem hiding this comment.
Maybe there is no need to state that it is an overridden method.
src/main/java/Parser.java
Outdated
| * Helps separate out key details in the input such as the task details and date. | ||
| * @throws DukeException When user input is wrong or incomplete. | ||
| */ | ||
| public void parse() throws DukeException { |
There was a problem hiding this comment.
Be wary when a method is longer than the computer screen, and take corrective action when it goes beyond 30 LOC (lines of code). The bigger the haystack, the harder it is to find a needle. - https://nus-cs2103-ay2021s1.github.io/website/se-book-adapted/chapters/codeQuality.html
src/main/java/Ui.java
Outdated
| * @param task Event details. | ||
| * @param date Date of event. | ||
| */ | ||
| public void event(String task, String date) { |
There was a problem hiding this comment.
Method names should contain verbs, seems like some other methods have this issue too
src/main/java/Duke.java
Outdated
| public static String logo = " ____ _ \n" | ||
| + "| _ \\ _ _| | _____ \n" | ||
| + "| | | | | | | |/ / _ \\\n" | ||
| + "| |_| | |_| | < __/\n" | ||
| + "|____/ \\__,_|_|\\_\\___|\n"; |
There was a problem hiding this comment.
The logo can variable can be set as a private static field.
src/main/java/Storage.java
Outdated
| private static final String DESTINATION = "./duke.txt"; | ||
| private final Path path; | ||
| private static String DESTINATION = "./duke.txt"; |
There was a problem hiding this comment.
Nice work. Constant names must be all uppercase using underscore to separate words.
src/main/java/Ui.java
Outdated
|
|
||
| storage.addData(list.getList()); | ||
| System.out.println(LINE); | ||
| storage.addData(list.getList()); | ||
|
|
||
| } catch (DukeException e) { | ||
| System.out.println(e.getMessage()); | ||
| System.out.println(LINE); | ||
| } | ||
| } catch (DukeException e) { | ||
| toReturn += e.getMessage(); | ||
| toReturn += "\n"; | ||
| } | ||
|
|
||
| sc.close(); | ||
| end(); | ||
| toReturn += LINE; | ||
| return toReturn; |
There was a problem hiding this comment.
This method is too long. Might make it very difficult to debug 🤔
src/main/java/TaskList.java
Outdated
| @@ -9,6 +9,9 @@ | |||
| private List<Task> list; | |||
There was a problem hiding this comment.
Plural form should be used on names representing a collection of objects.
src/main/java/Ui.java
Outdated
| @@ -182,28 +180,31 @@ public void event(String task, String date) { | |||
| * @param task Deadline details. | |||
| * @param date Date of deadline. | |||
| */ | |||
| public void deadline(String task, String date) { | |||
| public String deadline(String task, String date) { | |||
Add C-Priority
No description provided.