-
Notifications
You must be signed in to change notification settings - Fork 454
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
[Gabriel Waikin Loh Matienzo] iP #504
base: master
Are you sure you want to change the base?
Conversation
LUKE
LUKE helps you remember your tasks, events and deadlines for you!
And it is FREE! Features: Managing tasks
Here's the
|
src/main/java/Duke.java
Outdated
|
||
String response = scanner.nextLine(); | ||
|
||
while (!response.equals("bye")) { |
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 you should try splitting the logic in this while loop into different files/methods
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 code seems to adhere to the coding standard specified overall!
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 alright! Not many errors regarding the coding standards here!
src/main/java/Duke.java
Outdated
+ "|____/ \\__,_|_|\\_\\___|\n"; | ||
System.out.println("Hello from\n" + logo); | ||
|
||
ArrayList<String> list = new ArrayList<String>(); |
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.
Variable 'list' could be more descriptive; i.e, describe the purpose of the variable
src/main/java/Duke.java
Outdated
int taskNo = Integer.parseInt(response.replaceAll("\\D", "")) - 1; | ||
System.out.println(lineBreak); | ||
System.out.println("\t Nice! I've marked this task as done:"); | ||
list.set(taskNo, list.get(taskNo).substring(0,4) + "X" + list.get(taskNo).substring(5)); |
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.
substring(0,4)
substring(0, 4)
src/main/java/Duke.java
Outdated
private static void save(ArrayList<String> tasks) throws IOException { | ||
File f = new File("tasks.txt"); | ||
System.out.print(f.createNewFile()); | ||
FileWriter fw = new FileWriter("tasks.txt"); |
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.
A longer, more descriptive name (like simply fileWriter
) might be better here.
src/main/java/Duke.java
Outdated
list.add(amount, "[T][ ] " + response.substring(5)); | ||
amount++; | ||
save(list); | ||
} else if (response.matches("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.
For response matching, you might benefit from using Enums :)
src/main/java/Duke.java
Outdated
System.out.println("☹ OOPS!!! The description of an event cannot be empty."); | ||
System.out.println(lineBreak); | ||
} else { | ||
System.out.println(lineBreak); |
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 could consider abstracting the
System.out.println(lineBreak);
System.out.println(something);
System.out.println(lineBreak);
to avoid repeated code
Good code, but you might benefit from splitting your logic into multiple files :) |
Sample data has many blank lines. Blank lines are signs of messy code. Let's delete the blank lines. It is only way to get rid of the blank lines. This is an example for A-FullCommitMessage.
A-Assertions
A-CodeQuality
Add Gradle support
No description provided.