[Wang Haochen] iP #233
[Wang Haochen] iP #233
Conversation
add date time recognition
Add junit tests for DateValidator & Task classes
Add level-9 Find
add missing branch
Branch a coding standard
Fix incorrect branch name
src/main/java/ssagit/Duke.java
Outdated
| /** Create validator for dates */ | ||
| DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern("d/MM/yyyy HHmm", Locale.ENGLISH); | ||
| DateValidator validator = new DateValidatorLocalDate(dateFormatter); | ||
| /** Handler for all UI stuff. */ |
There was a problem hiding this comment.
I think adding an empty line between these logical blocks would help readability
src/main/java/ssagit/Duke.java
Outdated
| @@ -0,0 +1,36 @@ | |||
| package ssagit; | |||
There was a problem hiding this comment.
Seems a little odd that your package is called ssagit when the application is still called duke.
| /** | ||
| * Checks if date given is valid, in the following order: | ||
| * date - time - locale; | ||
| * If all 3 fails, return with invalid, otherwise return results. |
| public boolean isValid(String dateStr) { | ||
| LocalDateTime ldt = null; | ||
|
|
||
| try { |
There was a problem hiding this comment.
This is really hard to read, why do you need to catch the same kind of exception in so many different places? Maybe there's a way to have only one try-catch block?
| /** | ||
| * Handler for all UI displays of Duke.java | ||
| */ | ||
| public class ConsoleUI { |
There was a problem hiding this comment.
I think UI should be Ui (lowercase i)
| /** | ||
| * Exception class for missing todoTask descriptor. | ||
| */ | ||
| static class MissingTodoDescriptorException extends Exception { |
There was a problem hiding this comment.
I feel these exceptions should be under a different class, like a DukeException suggested in the project guide
| * @param tasks List of tasks from file. | ||
| * @param taskIterator Integer to count number of tasks at a time. | ||
| */ | ||
| public void parseInput(Task[] tasks, int taskIterator) { |
There was a problem hiding this comment.
Whew this is long, maybe try moving some logic to other methods.
| break; | ||
| case "todo": | ||
| case "event": | ||
| case "deadline": |
There was a problem hiding this comment.
This doesn't seem like good practice to me, should handle the todo and event in their respective cases I feel
test CI
add accessors
resolve issue with filePath (fP), remove FP assertion
Add file counter, add media resources
Update javafx and final cleanup
Pull request for week 3 iP