[Chew Song Yu] iP #113
[Chew Song Yu] iP #113
Conversation
Merge branch 'A-MoreOOP'
|
|
||
| public class Parser { |
There was a problem hiding this comment.
Should there be a javadoc description for the class? I noticed this in the other classes also.
| this.filePath = filePath; | ||
| } | ||
|
|
||
| public void printFileContents() { |
There was a problem hiding this comment.
Should there be a javadoc description for this public method? I noticed this in the other methods also.
| this.time = Integer.parseInt(dateAndTimeArr[1]); | ||
| } | ||
|
|
||
| public Deadlines(String task, String dateAndTime, boolean done) { |
There was a problem hiding this comment.
Perhaps the 'done' parameter should be renamed to 'isDone' to make it sound like a boolean. I saw this is some other methods as well.
| this.date = LocalDate.parse(dateAndTimeArr[0]); | ||
| this.time = Integer.parseInt(dateAndTimeArr[1]); |
There was a problem hiding this comment.
Is the 'this' keyword necessary here? The date and time variables are not shadowed by the constructor parameters so maybe it would be better to leave them out as per the java coding standard. I saw this is some other methods as well.
|
|
||
| @Override | ||
| public String toString() { | ||
| String doneIndicator = this.done ? "[✓]" : "[✗]"; |
There was a problem hiding this comment.
Would it be better to make "[✓]" and "[✗]" named constants to avoid using 'magic' values?
| System.out.println("\t\t" + task.toString()); | ||
| System.out.println("\tNow you have " + this.taskList.size() + " tasks in the list."); | ||
| Ui.sectionize(); | ||
| } catch (IndexOutOfBoundsException e) { |
There was a problem hiding this comment.
Maybe 'e' should be renamed to something more descriptive, eg. 'error'
src/main/java/com/duke/gui/Main.java
Outdated
| package com.duke.gui; | ||
| import java.io.IOException; |
There was a problem hiding this comment.
Should there be a line between the package and import statements?
| @@ -0,0 +1,133 @@ | |||
| package com.duke.parser; | |||
There was a problem hiding this comment.
I like how comprehensive your testing here is!
| public class ParserTest { | ||
|
|
||
| @Test | ||
| public void isDoneTest_inputLetters_returnFalse() { |
There was a problem hiding this comment.
I like the clear naming of the testing methods!
Add assertions
Refactor code
No description provided.