[Qu Mingsi] iP #468
[Qu Mingsi] iP #468
Conversation
Merge branch 'branch-Level-8' # Conflicts: # src/main/java/Duke.java
| * this method handles two types of error: invalid input (does not contain todo/deadline/event) | ||
| * and empty task (if the input starts with todo/deadline/event and the content is not empty, | ||
| * it is assumed that the input has correct structure) | ||
| */ |
There was a problem hiding this comment.
As per iP task on code quality, should the JavaDoc contain descriptions on the input parameter?
src/main/java/Duke.java
Outdated
| } | ||
| } | ||
|
|
||
| private static void readFile() throws FileNotFoundException { |
There was a problem hiding this comment.
As per iP task on code quality, should this function be accompanied with a JavaDoc?
AudreyFelicio
left a comment
AudreyFelicio
left a comment
There was a problem hiding this comment.
Overall, the code looks good and well structured 😃.
Only some small improvements needed.
Great job!
src/main/java/Deadline.java
Outdated
| return "[D]" + super.toString() + "(by: " + | ||
| time.format(DateTimeFormatter.ofPattern("MMM d yyyy HH:mm")) + ")"; |
There was a problem hiding this comment.
According to the module's coding standard, should the + after "(by: " be on the second line instead of the first line?
src/main/java/Event.java
Outdated
| return "[E]" + super.toString() + "(at: " + | ||
| time.format(DateTimeFormatter.ofPattern("MMM d yyyy HH:mm")) + ")"; |
There was a problem hiding this comment.
According to the module's coding standard, should the + after "(at: " be on the second line instead of the first line?
src/main/java/Parser.java
Outdated
| } | ||
|
|
||
| /** | ||
| * this method handles two types of error: invalid input (does not contain todo/deadline/event) |
There was a problem hiding this comment.
According to the module's coding standards, should the javadoc header comment start in the form of Handles ...?
src/main/java/Storage.java
Outdated
| try { | ||
| writeToFile(tasks); | ||
| } catch (IOException e) { | ||
| ui.showIOException(e); |
There was a problem hiding this comment.
According to the module's coding standards, should the method name showIOException be showIoException?
src/main/java/Ui.java
Outdated
| System.out.println("File not found"); | ||
| } | ||
|
|
||
| public static void showIOException(IOException e) { |
There was a problem hiding this comment.
According to the module's coding standards, should the method name showIOException be showIoException?
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; |
There was a problem hiding this comment.
According to the module's coding standards, should import static org.junit.jupiter.api.Assertions.assertEquals; be put above import org.junit.jupiter.api.Test;? The same comment for all other classes that have those two imports.
| public LocalDateTime getTime() { | ||
| return this.time; | ||
| } |
There was a problem hiding this comment.
According to the module's coding standards, should a javadoc comment exist above the method? The same comment for all other public methods.
| public void testSetDone(){ | ||
| DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd HHmm"); | ||
| Event event = new Event("return book", LocalDateTime.parse("2000-01-01 2359", formatter)); | ||
| assertEquals(false, event.getStatus()); |
There was a problem hiding this comment.
You can use assert(!event.getStatus()); here as well, just to make the code slightly cleaner, similar comment for line 20. The rest looks good, great job!
# Conflicts: # src/main/java/data/Duke.txt
Use assertions
Improve code quality
4 participants
No description provided.