-
Notifications
You must be signed in to change notification settings - Fork 73
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
[Jia Juin] iP #50
base: master
Are you sure you want to change the base?
[Jia Juin] iP #50
Conversation
src/main/java/Duke.java
Outdated
Scanner in = new Scanner (System.in); | ||
command = in.nextLine(); | ||
String option_1 = "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.
Your string name could be more precise. To illustrate what I mean, perhaps you could use commandByeWord?
For instance,
String commandByeWord = "bye";
src/main/java/Duke.java
Outdated
command = in.nextLine(); | ||
// if there is no "done", "bye", "list" in the string | ||
String splitter = in.nextLine(); | ||
// concat() method returns a String with the value of the String passed into the method, |
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 explanation of methods for non-technical users! Also, the indentation for comments is observed.
src/main/java/Duke.java
Outdated
{ | ||
return List.get(j); | ||
} |
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 brackets for your methods should follow the Egyptian style (aka K&R). I see you have followed the bracket coding style in Tasks.java.
To illustrate this,
while (!done) {
doSomething();
done = moreToDo();
}
src/main/java/Duke.java
Outdated
// cannot use in.nextLine() since it will treat (done (taskNumber)) as one string | ||
// thus will not lead us to the done statement | ||
command = in.next(); | ||
|
||
if (command.equals(option_1)) | ||
{ | ||
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.
Perhaps you can put the constant strings as constants. For example,
private static final String LINE_BREAK = "----------------------------------------";
src/main/java/Duke.java
Outdated
System.out.println("------------------------"); | ||
i++; | ||
command = in.nextLine(); | ||
// if there is no "done", "bye", "list" in the 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.
Good attempt of explaining choice of use of next line of code.
src/main/java/Task.java
Outdated
protected String description; | ||
protected boolean isDone; | ||
|
||
public Task(String description) { |
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 job! the names for the methods are well done.
* tag 'branch-Level-5': level-6 done
…es for different command for easier aceess.
A-UserGuide-done
No description provided.