-
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
[Liu Han] ip #500
base: master
Are you sure you want to change the base?
[Liu Han] ip #500
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.
Looks good. 👍
Some cosmetic changes are requested to follow coding standards.
LGTM.
src/main/java/Duke.java
Outdated
bye, | ||
list, | ||
mark, | ||
unmark, | ||
todo, | ||
delete |
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.
Enum names should be written in PascalCase or all uppercase?
BYE,
LIST,
...
src/main/java/Duke.java
Outdated
todo, | ||
delete | ||
} | ||
|
||
public static void main(String[] args) { | ||
String logo = " ____ _ \n" |
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.
I'm thinking that maybe variable logo can be set as final?
@@ -13,7 +13,7 @@ then | |||
fi | |||
|
|||
# compile the code into the bin folder, terminates if error occurred | |||
if ! javac -cp ../src/main/java -Xlint:none -d ../bin ../src/main/java/*.java | |||
if ! javac -cp ../src/main/java -Xlint:none -d ../bin ../src/main/java/Duke.java |
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.
I like how you changed the accessibility where it is more specific 😄
src/main/java/Duke.java
Outdated
public static void main(String[] args) { | ||
String logo = " ____ _ \n" | ||
+ "| _ \\ _ _| | _____ \n" | ||
+ "| | | | | | | |/ / _ \\\n" | ||
+ "| |_| | |_| | < __/\n" | ||
+ "|____/ \\__,_|_|\\_\\___|\n"; | ||
System.out.println("Hello from\n" + logo); | ||
String hello = "____________________________________________________________\n" + |
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.
I think can leave an empty space here?
src/main/java/Duke.java
Outdated
@@ -1,10 +1,53 @@ | |||
import java.util.*; |
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.
Import of classes should be listed explicitly?
src/main/java/Duke.java
Outdated
" Here are the tasks in your list:"; | ||
|
||
System.out.println(strlst); | ||
|
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.
I think can remove a blank line here 😃
Update all the codes to follow coding standards
correct the functions for level 2
Correct the functions and separate Task class from Duke class file
src/main/java/Duke.java
Outdated
" ____________________________________________________________\n"; | ||
System.out.println(hello); | ||
|
||
List<Task> list = new ArrayList<Task>(); |
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 can change this variable name to tasks or something like that to make it clearer?
src/main/java/Duke.java
Outdated
|
||
System.out.println(strlst); | ||
|
||
String todolist = "____________________________________________________________\n" + |
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.
Should this be todoList
?
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.
LGTM!
src/main/java/Duke.java
Outdated
int number = sc.nextInt(); | ||
Task task = list.get(number - 1); | ||
task.markAsDone(); | ||
System.out.println(" ____________________________________________________________\n" + |
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 consider placing line break before operator +
src/main/java/Duke.java
Outdated
|
||
|
||
|
||
String strlst = "____________________________________________________________\n" + |
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 can change this variable to stringList or something like that to make it clearer?
src/main/java/Duke.java
Outdated
public class Duke { | ||
|
||
enum Ability { |
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.
Good use of enums for different commands!
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've made good progress in your iP. All the best for future weeks
src/main/java/Duke.java
Outdated
int number = sc.nextInt(); | ||
Task task = list.get(number - 1); | ||
task.markAsDone(); | ||
System.out.println(" ____________________________________________________________\n" + |
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.
Also consider storing the separator as a constant
src/main/java/Duke.java
Outdated
String hello = " ____________________________________________________________\n" + | ||
" Hello! I'm Duke\n" + | ||
" What can I do for you?\n" + | ||
" ____________________________________________________________\n"; |
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.
Good minimalist design
src/main/java/Duke.java
Outdated
String input = sc.next(); | ||
|
||
while (!input.equals("bye")) { | ||
if (input.equals("list")) { |
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.
Consider using switch
case instead of if-else
Correct and supplement
Update and correct the codes
Add delete function
Save the tasks in the hard disk automatically whenever the task list changes.
Teach Duke how to understand dates and times. Store deadline dates as a java.time.LocalDate objects. Accept dates in yyyy-mm-dd format (e.g., 2019-10-15) and print in a different format MMM dd yyyy e.g., (Oct 15 2019).
Do it in a new branch
Update code in a new branch
it's for merge trial
Added Storage, Parser, TaskList classes
Package all classes into duke
Use Gradle to automate some of the build tasks of the project. Now I can run Duke.main() using Gradle but cannot run it in the terminal (If I do java Duke.java it will report 9 errors saying cannot find symbols and compilation failed. I don't know why.).
create a folder (test/java/duke) to place all the test classes, use Assertions to compare the expected and actual outcomes
Package the app as an executable JAR file so that it can be distributed easily.
Add JavaDoc comments to the code. Add header comments to all non-private classes/methods.
Tweak the code to comply with a coding standard.
Give users a way to find a task by searching for a keyword.
Place line break before operators.
Make some cosmetic changes to the codes according to the iP code quality feedback from teaching group
Add assert statement in printList method in Parser class
Improve the Javadoc statement for getStatus method
Use Assertions
Duke Helper
Introduction
Duke is an interactive personal chat robot designed for people tired of remembering all the tasks they've done or they're going to do. It's
Steps to use
Features
Addition
If you are a Java programmer, you can also use it to practice Java. Here's the
main
method: