-
Notifications
You must be signed in to change notification settings - Fork 362
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
[Lin Weilin] iP #368
base: master
Are you sure you want to change the base?
[Lin Weilin] iP #368
Conversation
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 coding standard has been followed, just apart from some minor issues! 👍 In most of your classes, do remember to keep fields private to prevent unwanted modifications by the user. It might also be good to declare Task as an abstract class since it should not be instantiated at any point in the program. Do let me know if you have any queries regarding my comments :)
src/main/java/Deadline.java
Outdated
String taskDescription; | ||
LocalDate deadLine; |
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 these class fields have corresponding getters and setters, it might be better to make them private to prevent any unintended modifications.
src/main/java/Duke.java
Outdated
System.out.println("Got it. I've added this task: \n " + task + | ||
"\nNow you have " + taskList.size() + " tasks in the list."); | ||
|
||
} else if(input.length() > 5 && (input.substring(0, 5)).equals("event")) { |
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.
Try to surround reserved words with a whitespace for consistency
src/main/java/Event.java
Outdated
String taskDescription; | ||
LocalDate startDate; | ||
LocalDate endDate; |
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.
Same goes for this, regarding making class fields private
src/main/java/Deadline.java
Outdated
@Override | ||
public String toString() { | ||
return "[D]" + super.toString() + " ( by: " + this.getDeadline() + " )" ; | ||
} |
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.
This is a good way to separate the different display behaviors of different task types 👍
Add A-Assertions
Parser performCommand: Code quality has not been achieved because method is too long and OOP is not fully achieved PerformCommand is supposed to handle the general processing of command, checking the performing comman shall be done in separate classes for each different cases. Created several classes in Parser to handle different types of commands to make the code quality better, ensuring methods are not too long and as well as to make full use of OOP.
…omised path for my hard disk The user using Duke might not have same file path in local hard disk The file path has thus been changed to ./data/duke.txt which makes it more general than the previous iteration Let's create a new text file in ./data/duke.txt instead of C:\Users\linwe\Documents\taskSaved.txt!
…of the validity of inputs are not robust Checking not being robust will result in Duke mis-intepereting an input as valid when it is not, thereby causes execption being thrown without catching Included more robust checking to ensure the inputs are in correct format, before a task object is created Let's check the inputs by trying throwing and catching exception when inputs are incorrect!
Same tasks can be added multiple times Add C-DetectDuplicates It is more user friendly so that users would not accidentally add same tasks into the task list. Let's * Check whether there are existing same task in task list as the task that user has keyed in * Include the equals function in task to check whether two tasks are the same or not
the interaction with User and not the parser
and improved implementation of the different tasks
Improve code style and quality and passed checkstyle
Entering dates before the current date is not allowed For event, start date cannot be after end date
Settle case when input is missing or is filled with spaces
BearyBear
BearyBear frees your mind of having to remember things you need to do. It's,
All you need to do is,
And it is FREE!
Features:
If you Java programmer, you can use it to practice Java too. Here's the main method: