-
Notifications
You must be signed in to change notification settings - Fork 270
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
[Justine Koh] iP #291
base: master
Are you sure you want to change the base?
[Justine Koh] iP #291
Conversation
Added Mark and Unmark functionalities.
Edited Add functionality of chatbot to support the above 3 task types.
…bugs when trying to add an event with invalid arguments.
src/main/java/Deadline.java
Outdated
@@ -0,0 +1,13 @@ | |||
public class Deadline extends 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.
Consider adding header comments for the class and its public methods?
src/main/java/Deadline.java
Outdated
@@ -0,0 +1,13 @@ | |||
public class Deadline extends Task { | |||
protected String by; |
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 using a noun here will be more meaningful?
src/main/java/Duke.java
Outdated
+ "| |_| | |_| | < __/\n" | ||
+ "|____/ \\__,_|_|\\_\\___|\n"; | ||
System.out.println("Hello from\n" + logo); | ||
String line = " ______________________________\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.
Consider making this a class-level constant?
src/main/java/Duke.java
Outdated
Scanner sc = new Scanner(System.in); | ||
List<Task> list = new ArrayList<>(); | ||
|
||
String cmd = sc.nextLine().strip(); |
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 full English word as a variable name here?
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.
Overall, I found your code easy to read.
Hello from | ||
____ _ | ||
| _ \ _ _| | _____ | ||
| | | | | | | |/ / _ \ | ||
| |_| | |_| | < __/ | ||
|____/ \__,_|_|\_\___| |
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 add a similar visual here.
src/main/java/bob/Triplet.java
Outdated
@@ -0,0 +1,29 @@ | |||
package bob; | |||
|
|||
public class Triplet<T, U, V> { |
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.
Cool to see you using generics here and making a triplet class. However, it could be more useful to name this class more purposefully, "Task"?
src/main/java/bob/Task.java
Outdated
public void toggleStatus() { | ||
this.isDone = !this.isDone; | ||
} |
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.
hmm, seems like this method is not used anywhere, perhaps you'll want to remove it?
public void toggleStatus() { | |
this.isDone = !this.isDone; | |
} | |
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.
Overall, I found your code easy to read for the most part, and see that you have written it well! LGTM, and keep up the good work.
} | ||
|
||
/** | ||
* Returns the size of the TaskList. |
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.
Nicely indented code, and following coding standards well. Like to see it! Keep it up!
src/main/java/bob/Parser.java
Outdated
public static void parse(String userInput, TaskList tasks, Storage storage) { | ||
try { | ||
String cmd = userInput.split(" ")[0].strip(); | ||
boolean tasksIsDiff = false; |
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 renaming tasksIsDiff to isTaskDiff would be better as a boolean name?
src/main/java/bob/Storage.java
Outdated
} | ||
|
||
/** | ||
* Converts a given list of tasks into a String and writes it to the file associated with this Storage instance. | ||
* | ||
* @param tasks List of tasks to be written to file. | ||
*/ | ||
public void writeTaskListToFile(TaskList tasks) { | ||
StringBuilder tasksString = new StringBuilder(); | ||
for (int i = 0; i < tasks.size(); i++) { | ||
tasksString.append(tasks.get(i).generateSavedEntry()); | ||
tasksString.append("\n"); | ||
} | ||
try { | ||
FileWriter fileWriter = new FileWriter(file); | ||
fileWriter.write(tasksString.toString()); | ||
fileWriter.close(); | ||
} catch (IOException e) { | ||
System.out.println("Error writing to file!:( : " + e.getMessage()); | ||
} | ||
} | ||
} |
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 that you have well-written javadocs, it makes code easier to read and understand!
There are multiple conditions that must hold true at every point of the program flow. If any of these conditions do not hold true when they should, it will cause unexpected behaviour in the program. Having assertions in the code to guarantee such conditions gives us confidence that the program is running smoothly with all necessary conditions met at all assertion checkpoints if no assertions fail.
Code quality is important for readability and to facilitate maintenance of code. Long methods have been shortened, by abstracting out suitable functional blocks of code, and in accordance to SLAP. Nested indentations have been removed and cleaned up. Updated most JavaDocs.
* master: Add .yml file for Continuous Integration
branch-A-Assertions
* 'master' of https://github.com/justinekoh/ip: Add assertions to check for necessary conditions in program flow
* master: Add assertions to check for necessary conditions in program flow
Improve code quality
…-A-CodeQuality * 'master' of https://github.com/justinekoh/ip:
* 'master' of https://github.com/justinekoh/ip: Improve code quality
* branch-A-CodeQuality:
Add command and archive functionalities
Changed Bob and user photos, made Bob and user photos round, added title to application
Increased size of window, disabled window resizing and fixed bugs with displayed text.
BoB
BoB frees your mind of having to remember things you need to do. It's
FASTSUPER FAST to useAll you need to do is,
And it is FREE!
Features:
If you are a Java programmer, you can use it to practice Java too. Here's the
main
method: