-
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
[immanuelhume] iP #32
base: master
Are you sure you want to change the base?
Conversation
Also renamed Duke.java to Chungus.java and created a new Task.java file.
Also organized imports
Also resolved some conflicts and issues with marshalling datetimes
Does not provide much benefit over a regular ArrayList for now
It was overriden to tabs somehow
Also created a new chungus.util package
Also added some more JavaDocs
Now each Chungus instance can use any Ui instance. Later, when adding a GUI in JavaFX, our lives would be easier.
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.
Appreciate the commitment to chonciness 👏 👏
We mainly assert some facts about the sizes of strings when unmarshalling data. Also, the static methods on the Handler class was renamed to sound more like a verb.
This better describes its functionality, since it is important that the "init" method does not block. Callers are expected to block the application in their own event loop.
We also do a check that the dimensions in fxml match those expected in code.
Add some assertions
Improve code quality
Because I want to save my free tier minutes.
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 code looks gr8 with very detailed documentation. Just some minor formatting and organization issues. Consider separating the io, exceptions, tasks into different packages.
/** | ||
* A todo. | ||
*/ | ||
class Todo 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.
Perhaps put the different tasks in separate files? And then put them all in one separate package.
* @return An appropriate handler for the command. | ||
* @throws ChungusException When the expected command format is wrong. | ||
*/ | ||
public static Handler parse(String cmd) { |
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.
Very long method. Perhaps abstract away the parsing so that it is easier to maintain and add new cases. But nice use of switch cases!
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 this is fine for now as the length comes from the number of cases. Within each case we only extract some values and pass it to the handler so it's not actually doing much. Thanks for the suggestion though!
private NonBlockingUi ui; | ||
private TaskList tasks; | ||
private Storage db; | ||
private AtomicBoolean isRunning; | ||
|
||
public static final String DEFAULT_DB_PATH = System.getProperty("user.dir") + "/chungus.db"; |
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 the static attributes should be above the private final attributes according to coding standards
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.
In which page did you find this? I wasn't able to find it at https://se-education.org/guides/conventions/java/intermediate.html
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.
Tbh idk someone suggested that to me too.
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.
lol
class TaskNotFoundException extends ChungusException { | ||
public TaskNotFoundException(int idx) { | ||
super(String.format("Task %d does not exist", idx)); | ||
} | ||
} | ||
|
||
/** | ||
* An exception to represent when the serialization of a task is invalid. | ||
*/ | ||
class TaskMarshalException extends ChungusException { | ||
public TaskMarshalException(String s) { | ||
super(String.format("Bad marshal format: \"%s\"", s)); | ||
} | ||
} |
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.
Same as tasks, maybe put these into separate files and put all the exceptions into one package.
private String desc; | ||
private boolean isDone; | ||
|
||
protected HashSet<String> tags; | ||
|
||
protected static final DateTimeFormatter DATETIME_FMT = DateTimeFormatter.ISO_LOCAL_DATE_TIME; |
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.
Same here, perhaps the static attributes should be above the private final attributes according to coding standards
Just some rephrasing and disambiguation.
Chungus
Chungus (not to be confused with Big Chungus) is a simple task tracking assistant. Chungus is
Basic Usage
java -jar chungus.jar
Reading the Code
The entry point can be found in
Chungus.java
.Each Chungus instance expects an input and output stream, as well as the path to a file on disk to store its data. By default, it uses
./chungus.db
, relative to the current working directory.Feature Roadmap