-
Notifications
You must be signed in to change notification settings - Fork 364
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
[cydtseng] iP #136
base: master
Are you sure you want to change the base?
[cydtseng] iP #136
Conversation
# Conflicts: # src/main/java/Deadline.java # src/main/java/Duke.java # src/main/java/Event.java
# Conflicts: # src/main/java/duke/Deadline.java # src/main/java/duke/Duke.java # src/main/java/duke/DukeException.java # src/main/java/duke/Event.java # src/main/java/duke/Todo.java # src/main/java/duke/Ui.java
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 except some minor nitpicks. Very clean code!
|
||
public static void saveTasksToFile(ArrayList<Task> taskList) { | ||
try { | ||
String toWrite = ""; |
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.
A little pedantic, but it might be better to use a noun for this String, such as tasksToWrite
src/test/java/ParserTest.java
Outdated
public void testParseByeCommand(){ | ||
Assertions.assertTrue(parser.isByeCommand("bye")); | ||
Assertions.assertTrue(parser.isByeCommand("bye ")); | ||
Assertions.assertTrue(parser.isByeCommand(" bye")); |
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.
It's great that you're testing these edge cases!
public class Parser { | ||
private final Stage stage; | ||
|
||
public enum CmdType { |
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.
enum constants should be in SCREAMING_SNAKE_CASE
src/main/java/duke/Parser.java
Outdated
return userInput.split(" ")[0].equals(CmdType.find.name()); | ||
} | ||
|
||
public String parseCommandWithResponse(String userInput) { |
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 method seems way too long. As a suggestion, perhaps you could break your method up into handleByeCommand, handleMarkCommand etc so it's easier to debug.
For now, assertions have been added to the DialogBox class and the Storage class. The assertions are to check assumptions of non null imageviews and dialogbox labels as well as storage file directory and saved task file existence. This was referenced in enabling assertions https://se-education.org/guides/tutorials/intellijUsefulSettings.html
Update project with assertions enabled for intellij and gradle
Refactor some code that does not adhere to SLAP
Add help command to show all possible command formats for users
colors and fonts
Duke Project
The Duke Project is a chatbot based todo list with the following features
How to run:
Interactivity progress:
Entry point
main
of Duke program.