-
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
[T1duS] ip #680
base: master
Are you sure you want to change the base?
[T1duS] ip #680
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.
src/main/java/Opus.java
Outdated
String s = scanner.nextLine(); | ||
String[] words = s.split(" "); | ||
|
||
try{ |
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 use egyptian style brackets?
src/main/java/Opus.java
Outdated
else if (words[0].equals("delete")) { | ||
int i = Integer.parseInt(words[1]); | ||
System.out.println("Noted. I've removed this task:"); | ||
System.out.println(" " + tasks[i]); | ||
for (int j = i; j < taskCount; j++) { | ||
tasks[j] = tasks[j + 1]; | ||
} | ||
taskCount--; | ||
System.out.println(" Now you have " + taskCount + " tasks in the list."); |
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.
Would it be better to extract out the tasks into individual classes? Perhaps it will make the code look cleaner
src/main/java/Opus.java
Outdated
tasks[++taskCount] = new Deadline(parts[0], parts[1]); | ||
} else if (words[0].equals("event")) { | ||
String[] parts = s.substring(6).split(" /from "); | ||
String[] parts2 = parts[1].split(" /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.
Perhaps a more intuitive variable name here?
src/main/java/Deadline.java
Outdated
@@ -1,13 +1,18 @@ | |||
public class Deadline extends Task { | |||
protected String by; | |||
private 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 can give the deadline date a clearer name? Right now the name "by" may not be clear to other programmers.
src/main/java/Parser.java
Outdated
@@ -0,0 +1,3 @@ | |||
public class Parser{ |
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.
This is nitpicking but don't forget the space to follow coding standards. In the future can check through.
src/main/java/ToDo.java
Outdated
@Override | ||
public String toString() { | ||
return "[T]" + super.toString(); | ||
return "[T]" + (isDone ? "[X] " : "[ ] ") + 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.
Perhaps you could add a method to Task class that returns a string with (isDone ? "[X] " : "[ ] "). Then you can call super.thisMethod for easier coding?
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 good in general.
However, there are a few coding standard violations and potential areas for improvement.
src/main/java/Storage.java
Outdated
writer.close(); | ||
} | ||
catch (IOException e){ | ||
return; |
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 logging or printing an error message could be helpful for diagnosing the exceptions here?
src/main/java/Opus.java
Outdated
taskList = new TaskList(storage.load()); | ||
} | ||
|
||
public void run() { |
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.
Could this method be refactored into smaller methods to handle the different scenarios more effectively?
Branch-level-10
A-checkstyle
A-Assertions
updated docs
finished userguide
BCD-Extension
A-CodeQuality
Refactor Opus class to remove Ui dependency and add new command handlers * The delete, mark, help, and bye command have been seperately added as command handlers. * Javadoc comments have been added for each of these handle command functions * The previous run method which used to execute these commands and use the UI class has been removed. Now the UI used is the JavaFX one. So, the commit also removed the UI class because it is not needed anymore.
What this commit does:
code quality and exception changes
Description
This is Duke project called Opus.
Opus is a chatbot that does the following:
Features
Opus has the following features:
It is like your own personal calendar
If you are a Java programmer, you can use it to practice Java too. Here's the main method:
👍 Hope you enjoy using Opus