-
Notifications
You must be signed in to change notification settings - Fork 590
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
[samriddh2145] iP #645
base: master
Are you sure you want to change the base?
[samriddh2145] iP #645
Conversation
In build.gradle, the dependencies on distZip and/or distTar causes the shadowJar task to generate a second JAR file for which the mainClass.set("seedu.duke.Duke") does not take effect. Hence, this additional JAR file cannot be run. For this product, there is no need to generate a second JAR file to begin with. Let's remove this dependency from the build.gradle to prevent the shadowJar task from generating the extra JAR file.
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. Perhaps you may have repeated some code, which you can easily fix!
src/main/java/Bing.java
Outdated
System.out.println("______________________________\n"); | ||
} else if (input.startsWith("mark")) { | ||
int x = Integer.parseInt(input.substring(5)); | ||
if (x>0 && x<=tasks.size()) { |
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 may use some spacing around the characters here?
src/main/java/Bing.java
Outdated
|
||
} else if (input.startsWith("unmark")) { | ||
int x = Integer.parseInt(input.substring(7)); | ||
if (x>0 && x<=tasks.size()) { |
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 may use some spacing around the characters here too?
src/main/java/Bing.java
Outdated
String message = "______________________________\n" | ||
+ "Hi! My name is Bing\n" | ||
+ "How can I help you?\n" | ||
+ "______________________________\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.
I notice you have many areas in your code where you are using a string of underscores to denote a line. You may consider declaring a variable, e.g. LINE, to avoid code repetition!
src/main/java/Event.java
Outdated
this.to = to; | ||
} | ||
|
||
|
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.
Nit: Unnecessary newline
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, some refactoring & adding of JavaDoc comments seems to be required for code readability. Otherwise, looks good to merge!
src/main/java/Task.java
Outdated
private String info; | ||
private TaskStatus isDone; | ||
|
||
public Task(String info) { |
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 a JavaDoc comment to the explain constructor for this class.
src/main/java/TaskStatus.java
Outdated
this.StatusSymbol = StatusSymbol; | ||
} | ||
|
||
public String getStatusSymbol() { |
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 a Javadoc comment for this. Specifically, what is the status that is returned about.
src/main/java/Deadline.java
Outdated
|
||
private String deadline; | ||
|
||
public Deadline(String info, String 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.
Consider adding a JavaDoc comment for this as it will likely cause a Checkstyle error, since public constructors are required to have such a comment.
src/main/java/Bing.java
Outdated
import java.util.Scanner; | ||
import java.util.ArrayList; | ||
public class Bing { | ||
public static void main(String[] args) { |
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 abstracting away portions of your code to helper methods and then refactoring your code to match the OOP style of coding. Kudos on the extensive error handling nonetheless!
Add features to A-Assertions
Add changes to improve code quality
Implemented a new 'stats' command that provides users with insights into their tasks. The statistics include the total number of tasks, the number of marked and the number of unmarked tasks.
Bing - Your Personal Task Manager
Bing helps you manage your tasks effectively, so you can focus on what truly matters. It's:
All you need to do is:
Features:
Code Example: