-
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
[bryancheny] iP #624
base: master
Are you sure you want to change the base?
[bryancheny] iP #624
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, the code followed most of the coding conventions. I noted that you decided to write nested classes inside Task currently. Maybe you could consider abstracting them out instead if the classes start to get longer.
src/main/java/Crack.java
Outdated
@@ -0,0 +1,196 @@ | |||
import java.util.Scanner; | |||
import java.io.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.
Should the imports be done in lexicographical order?
src/main/java/Crack.java
Outdated
} else { | ||
System.out.println(divider + " Error: Task number out of range.\n" + divider); | ||
} | ||
} catch (Exception e) { |
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 we can be more specific with the Exception being caught here?
src/main/java/Crack.java
Outdated
while (fileScanner.hasNextLine()) { | ||
String line = fileScanner.nextLine(); | ||
String[] parts = line.split(" \\| "); | ||
switch (parts[0]) { |
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.
Should the indentation of the "cases" be in line with "switch"?
src/main/java/Task.java
Outdated
public String toString() { | ||
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("MMM dd yyyy"); | ||
return super.toString() + " (from: " + from.format(formatter) + " to: " + to.format(formatter) + ")"; | ||
} |
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 the naming of the variables and methods.
src/main/java/Task.java
Outdated
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("MMM dd yyyy"); | ||
return super.toString() + " (from: " + from.format(formatter) + " to: " + to.format(formatter) + ")"; | ||
} | ||
} |
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 consider adding some javadocs in your code?
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.
Apart from some project structure issues, LGTM
src/main/java/Task.java
Outdated
} | ||
} | ||
|
||
class Event 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.
I think putting this in another file would make the code neater.
Previously there was no GUI, only CMD-based user interaction. GUI is more user friendly and we chose JavaFX as our GUI based on its ease of use.
Integrated Java's assert statements across multiple classes to enforce important assumptions Included assertions to verify various task assertions were not null
Before this, there were 2 style errors raised by stylecheck. Caused gradle build to display fail message This commit fixes the style issues in Parser.java accordingly.
…ndards Applied the Single Level of Abstraction Principle (SLAP): - Extracted command handling logic from the run() method into dedicated methods such as handleMarkCommand(), handleTodoCommand(), handleDeadlineCommand(), etc. - This refactoring reduces the length of the run() method, making it more concise and easier to read. - Enhances readability by ensuring the run() method operates at a single level of abstraction. Updated Method Header Comments to comply with coding standard.
Merge branch A-Assertions
Merge branch A-CodeQuality
Added Snooze Command (B-Snooze): Implemented the ability to snooze or postpone tasks by updating their dates. Users can now snooze deadlines or events to postpone them with the snooze command. Refactored Crack Class: Removed the unused run() method since the application is now GUI-based. Refactored the getResponse() method to follow the SLAP method.
README was outdated and so updated it to include how to run and credits.
Previously, there was no user guide and new users could be lost on how Crack works. - Updated Crack with a new user guide. - Listed the features of Crack in the user guide.
Crack Chatbot Project
Crack is a simple task management chatbot application that helps users manage their tasks efficiently.
Features
How to Use
./gradlew clean build
./gradlew run
Development Tasks
This is a fork of this github repository