-
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
[Rama Venkatesh] iP #503
base: master
Are you sure you want to change the base?
[Rama Venkatesh] iP #503
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.
LGTM! Just some nits on coding standards and then it should be fine.
src/main/java/Duke.java
Outdated
System.out.println(" Noted. I've removed this task: "); | ||
System.out.println(" " + tasktoDel.toString()); | ||
} | ||
else if(input.substring(0,8).equals("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.
else if(input.substring(0,8).equals("deadline")){ | |
else if(input.substring(0,8).equals("deadline")) { |
src/main/java/Duke.java
Outdated
System.out.println("Got it. I've added this task: "); | ||
System.out.print(" " + newTask.toString()); | ||
|
||
} else if(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.
} else if(input.substring(0,5).equals("event")){ | |
} else if(input.substring(0,5).equals("event")) { |
src/main/java/Duke.java
Outdated
if(input.length()<4 ){ | ||
throw new DukeException("Unacceptable input"); | ||
} | ||
if(input.substring(0,4).equals("todo")){ |
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.
if(input.substring(0,4).equals("todo")){ | |
if(input.substring(0,4).equals("todo")) { |
src/main/java/Duke.java
Outdated
import tasks.*; | ||
import exceptions.*; |
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 can name out all the files that you are importing instead of using a wildcard?
src/main/java/Duke.java
Outdated
for(int i = 0; i < taskCounter; i++){ | ||
System.out.println((i+1) + "." + tasks.get(i).toString()); | ||
} | ||
} else if (input.length() > 4 && input.substring(0,4).equals("done")){ |
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.
} else if (input.length() > 4 && input.substring(0,4).equals("done")){ | |
} else if (input.length() > 4 && input.substring(0,4).equals("done")) { |
src/main/java/Duke.java
Outdated
} | ||
} else if (input.length() > 4 && input.substring(0,4).equals("done")){ | ||
String taskDone = input.substring(5); | ||
int taskDoneIndex = Integer.parseInt(taskDone)-1; |
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.
int taskDoneIndex = Integer.parseInt(taskDone)-1; | |
int taskDoneIndex = Integer.parseInt(taskDone) - 1; |
src/main/java/Duke.java
Outdated
System.out.println(tasks.get(taskDoneIndex).toString()); | ||
} | ||
else { | ||
if(input.length()<4 ){ |
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.
if(input.length()<4 ){ | |
if (input.length() < 4) { |
src/main/java/Duke.java
Outdated
taskCounter++; | ||
System.out.println("Got it. I've added this task: "); | ||
System.out.println(" " + newEvent.toString()); | ||
} else if(input.length()>5 && input.length()<8){ |
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.
} else if(input.length()>5 && input.length()<8){ | |
} else if (input.length() > 5 && input.length() < 8) { |
src/main/java/Duke.java
Outdated
} else if(input.length()>5 && input.length()<8){ | ||
throw new DukeException("Unacceptable input"); | ||
} | ||
else if(input.substring(0,6).equals("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.
else if(input.substring(0,6).equals("delete")) { | |
else if (input.substring(0,6).equals("delete")) { |
src/main/java/Duke.java
Outdated
}} catch (DukeException e){ | ||
System.out.println("OOPS!!! You have enteted an invalid category"); | ||
} |
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 format the curly braces better?
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 ok! I think you could work on the formatting of the braces as well as including whitespace in your code. Good luck!
} | ||
|
||
@Override | ||
public String toString(){ |
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.
public String toString() {
@Override | ||
public String toString(){ | ||
String typeString = type == TaskType.TODO ? "T" : type == TaskType.EVENT? "E" : "D"; | ||
String doneSymbol = isDone? "X" : " "; |
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.
String doneSymbol = isDone ? "X" : " ";
src/main/java/Tasks/EventTask.java
Outdated
@Override | ||
public String toString(){ | ||
String typeString = type == TaskType.TODO ? "T" : type == TaskType.EVENT? "E" : "D"; | ||
String doneSymbol = isDone? "X" : " "; |
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.
String doneSymbol = isDone ? "X" : " ";
Added DukeDate parse
Branch a coding standard
added javadocs
added find functionality
Add Gradle support pull req
added gradle support
modified gradle build file
No assert statements are present. Add statements are needed to declare for things that ought to be true at that point in time
Add assert statements
Find command is case sensitive A case insensitive find is more user-friendly because users cannot be expected to remember the exact case of the keywords Let's, * update the search algorithm to use case-insensitive matching
Find command: make matching case insensitive
Coding standard not fully adhered to and SLAP principle not followed in Ui class Adhering more closely to coding standards makes code more readable and better understood, and following coding principles makes it easier for other programmers to understand it. Let's * fix the order of import statements * follow SLAP principle in Ui class' init method * simplify complicated expressions in FindCommand class
Improve code quality
Duke
Duke frees your mind of having to remember things you need to do. It is:
FASTSUPER FAST to useAll 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: