-
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
[Fong Yi Fei] iP #282
base: master
Are you sure you want to change the base?
[Fong Yi Fei] iP #282
Conversation
…and cleaned up print statements in Bobby.
…ceptions and are handled in the main method
# Conflicts: # src/main/java/Deadline.java
# Conflicts: # src/main/java/bobby/Bobby.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.
Look pretty solid, some lines need to change according to the coding standard. Overall, good job 💯
|
||
@Override | ||
public void writeToFile(FileWriter fw) throws IOException { | ||
fw.write("E ; " + isDone + " ; " + description + " ; " + at + System.lineSeparator()); |
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 break line be 8 spaces indent compared to the parent line?
src/main/java/bobby/Deadline.java
Outdated
import java.time.format.DateTimeFormatter; | ||
|
||
public class Deadline extends Task { | ||
protected LocalDate 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.
Maybe another more meaningful name?
src/main/java/bobby/Parser.java
Outdated
case DEADLINE: | ||
try { | ||
tasks.addDeadline(userInput); | ||
} catch (BobbyException 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.
I think you can combine all the expected exceptions into one file and call that instance
src/main/java/bobby/TaskList.java
Outdated
String description = splitInputs[0]; | ||
LocalDate by = LocalDate.parse(splitInputs[1]); | ||
Deadline newDeadline = new Deadline(description, by); | ||
System.out.println("Bobby heard: " + newDeadline); |
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 take all the System.out.println
line into a new method in Ui class?
|
||
public class BobbyException extends Exception { | ||
public BobbyException(String errorMessage) { | ||
super(errorMessage); |
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 you can turn this into a more customize file, instead of simply extends the Exception class
src/main/java/bobby/Deadline.java
Outdated
|
||
@Override | ||
public String toString() { | ||
return "[D]" + super.toString() + " (by: " + by.format(DateTimeFormatter.ofPattern("MMM d yyyy")) + ")"; |
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 break line be 8 spaces indent compared to the parent line?
src/main/java/bobby/Parser.java
Outdated
} catch (BobbyException e) { | ||
System.out.println(e); | ||
} catch (DateTimeParseException e) { | ||
System.out.println("Invalid date format. Please use YYYY-MM-DD."); |
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 noticed the same issue in several other places too
…() method in Bobby
…ye" command Change GUI display pictures Ensure text is displayed fully for long tasklists
To ensure assumptions and ease of debugging for future extensions Let's insert assert statements in various points to ensure assumptions hold and methods function as expected
Branch a assertions
…odeQuality # Conflicts: # src/main/java/bobby/Parser.java
This reverts commit 5b234ed.
Parse() method in Parser class do not have same level of abstraction and have common behaviors across several switch cases. Extracting out common behaviors into private methods in the class improves the code quality of the method as it now has single level of abstraction and do not have common behavior across cases. Let's extract out the splitting process into various private methods to split the input
Branch a code quality
Undo command will help users correct their incorrect inputs easily. Let's keep track of the state of Bobby after each change made and implement the Undo command to reverse the most recent change.
It's Bobby.
Features
What can Bobby remember?
Here is how Bobby would encourage you in
taskDone
An example of what you could do with Bobby.
?