-
Notifications
You must be signed in to change notification settings - Fork 78
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
[lckjosh] iP #74
base: master
Are you sure you want to change the base?
[lckjosh] iP #74
Conversation
src/main/java/Duke.java
Outdated
switch (command) { | ||
case "list": | ||
if (noOfTasks == 0) { | ||
System.out.println("____________________________________________________________"); |
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 a constant / final variable to represent the line divider
src/main/java/Duke.java
Outdated
int taskToMark = Integer.parseInt(parameters) - 1; | ||
tasks[taskToMark].markAsDone(); | ||
System.out.println("____________________________________________________________"); | ||
System.out.println("I've marked this task as done:"); | ||
System.out.println(tasks[taskToMark].getStatusIcon() + " " + tasks[taskToMark].getDescription()); | ||
System.out.println("____________________________________________________________"); |
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 moving code in cases to another function to reduce verbosity
src/main/java/Duke.java
Outdated
String parameters = ""; | ||
Task[] tasks = new Task[100]; | ||
int noOfTasks = 0; | ||
while (true) { |
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.
May cause an unintended program behaviour, consider setting the loop conditional to while the command is not bye
src/main/java/Duke.java
Outdated
System.out.println("____________________________________________________________"); | ||
System.out.println("Goodbye, hope to see you again soon!"); | ||
System.out.println("____________________________________________________________"); | ||
in.close(); |
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.
Shift this to outside the loop and the condition becomes the loop condition
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.
Your code overall follows the coding standards and naming conventions. The only suggestion I have would to leave blank lines at certain points to separate out logic and make your code more readable.
src/main/java/BotBuddy/Duke.java
Outdated
printUnderscores(); | ||
System.out.println("Hello from BotBuddy!"); | ||
System.out.println("What can I do for you?"); | ||
printUnderscores(); | ||
Task[] tasks = new Task[100]; | ||
String input; | ||
String[] inputArr; | ||
String command = ""; | ||
String parameters = ""; | ||
Scanner in = new Scanner(System.in); |
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.
Perhaps you could leave a blank line between certain lines to seperate related logic (e.g. a blank line after your print statements)
package BotBuddy; | ||
|
||
public class BotBuddyException extends Exception { | ||
public BotBuddyException(String s) { |
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.
Perhaps you could use a more descriptive variable than s
such as inputString
add ability to find tasks
edit javadoc
edit javadocs
No description provided.