[Guo Jingxue] iP#237
Conversation
This reverts commit 44763c2.
1. ToDos: tasks without any date/time attached to it e.g., visit new theme park 2. Deadlines: tasks that need to be done before a specific date/time e.g., submit report by 11/10/2019 5pm 3. Events: tasks that start at a specific time and ends at a specific time e.g., team project meeting on 2/10/2019 2-4pm
…er the task list changes - Add ability to load the data from the hard disk when Duke starts up
- Add print statement if the list is empty
ming-00
left a comment
ming-00
left a comment
There was a problem hiding this comment.
LGTM! Few nits to work out, and the main method may be way too long.
| while (sc1.hasNextLine()) { | ||
| data = sc1.nextLine(); | ||
| String[] dataSplit = data.split(" \\| "); | ||
| switch (dataSplit[0]) { |
|
|
||
| @Override | ||
| public String toString() { | ||
|
|
| + "| |_| | |_| | < __/\n" | ||
| + "|____/ \\__,_|_|\\_\\___|\n"; | ||
| System.out.println("Hello from\n" + logo); | ||
| int counter = 0; |
There was a problem hiding this comment.
You should avoid overly long methods that take up more than the space in a computer screen
garyljj
left a comment
garyljj
left a comment
There was a problem hiding this comment.
Hi! Overall looks not bad!
No problem with any variable names 👍 But perhaps can private attributes for better encapsulation.
And also, since main method is pretty long, might be good idea to use blank lines to split up logical sections for easier readability!
| import java.time.format.DateTimeFormatter; | ||
|
|
||
| public class Deadlines extends Task { | ||
| LocalDate deadLine; |
There was a problem hiding this comment.
Don't forget to private attributes for encapsulation 👍
| @@ -0,0 +1,11 @@ | |||
| public class ToDos extends Task { | |||
There was a problem hiding this comment.
Perhaps use singular form (ToDo) since plural form usually represents a collection of objects, eg. List
| switch (inputSplit[0]){ | ||
| case "done": | ||
| if (inputSplit.length < 2){ |
There was a problem hiding this comment.
Very minor but remember to leave white space before "{" just for consistency and readability 😄
|
|
||
| @Override | ||
| public String toString() { | ||
| return "[D] " + super.toString() + "(by: " + deadLine.format(DateTimeFormatter.ofPattern("MMM d yyyy")) + ")"; |
There was a problem hiding this comment.
Perhaps you could try having this over two lines?
Quoting the java coding standards:
Try to keep line length shorter than 110 characters (soft limit). But it is OK to exceed the limit slightly (hard limit: 120 chars).
I think the line currently is 110+ characters, but it may improve readability slightly if placed over two lines.
| this.eventName = eventName; | ||
| this.eventType = eventType; | ||
| } | ||
| public void setDone() { |
There was a problem hiding this comment.
Might be good to add an empty line between the class constructor and the method, so that there is better readability?
| String time; | ||
|
|
||
| public Events(boolean isDone, String eventName, String time) { | ||
|
|
There was a problem hiding this comment.
I see an empty line here, but not in your constructors for your other classes. Perhaps you could remove this empty line to make it consistent with the rest?
|
Hi, very clear code overall! |
Improve code quality
Use Assertions
Allow command to be in lower or upper case or mixed
Update user guide
4 participants
No description provided.