-
Notifications
You must be signed in to change notification settings - Fork 461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Sien Soon] iP #505
base: master
Are you sure you want to change the base?
[Sien Soon] iP #505
Conversation
Edited EXPECTED.TXT for TextUiTesting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall your code looks fine, although could probably use a bit more abstraction for future implementations.
src/main/java/Deadline.java
Outdated
|
||
public class Deadline extends Task { | ||
|
||
protected LocalDateTime by; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you could name it as "deadlineTime" instead of "by"?
src/main/java/Duke.java
Outdated
duke.greeting(); | ||
duke.run(); | ||
duke.goodBye(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you could name the methods as verbs like greet() and exit() instead?
src/main/java/Duke.java
Outdated
case "T": { | ||
Task task = new ToDo(taskInArray[2]); | ||
taskList.add(task); | ||
if (taskInArray[1].equals("1")) { | ||
task.markAsDone(); | ||
} | ||
break; | ||
} | ||
|
||
case "D": { | ||
Task task = new Deadline(taskInArray[2], taskInArray[3]); | ||
taskList.add(task); | ||
if (taskInArray[1].equals("1")) { | ||
task.markAsDone(); | ||
} | ||
break; | ||
} | ||
|
||
case "E": { | ||
Task task = new Event(taskInArray[2], taskInArray[3]); | ||
taskList.add(task); | ||
if (taskInArray[1].equals("1")) { | ||
task.markAsDone(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since in each case you are marking the task as done when loading them, maybe u can abstract the process out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, the code looks clean and understandable. Just a few touching up of the formatting and it should be all good
src/main/java/Duke.java
Outdated
public class Duke { | ||
|
||
private final String horizontalLine = "-------------------------"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want the name of your constant variables like this one to match the coding standard. Perhaps something like:
private final String horizontalLine = "-------------------------"; | |
private final String HORIZONTAL_LINE = "-------------------------"; |
src/main/java/Duke.java
Outdated
switch (taskType) { | ||
case "T": { | ||
Task task = new ToDo(taskInArray[2]); | ||
taskList.add(task); | ||
if (taskInArray[1].equals("1")) { | ||
task.markAsDone(); | ||
} | ||
break; | ||
} | ||
|
||
case "D": { | ||
Task task = new Deadline(taskInArray[2], taskInArray[3]); | ||
taskList.add(task); | ||
if (taskInArray[1].equals("1")) { | ||
task.markAsDone(); | ||
} | ||
break; | ||
} | ||
|
||
case "E": { | ||
Task task = new Event(taskInArray[2], taskInArray[3]); | ||
taskList.add(task); | ||
if (taskInArray[1].equals("1")) { | ||
task.markAsDone(); | ||
} | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation for each case statement should be aligned to the switch
# Conflicts: # src/main/java/duke/TaskList.java
To allow Duke to understand dates and times instead of string for event schedule. Store dates and times in format (yyyy-mm-ddThh:mm:ss) and prints in a different format (MMM dd yyyy hh:mm:ss).
Exits Duke when "bye" is entered by user
…mark, unmark, delete and find is performed
DukePro
DukePro frees your mind of having to remember things you need to do. It's,
FASTSUPER FAST to useAll you need to do is,
And it is FREE!
Features: