Skip to content
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

[Isaac] iP #496

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

[Isaac] iP #496

wants to merge 30 commits into from

Conversation

Isaaclhy00
Copy link

@Isaaclhy00 Isaaclhy00 commented Aug 28, 2022

Duke

“Productivity is never an accident. It is always the result of a commitment to excellence, intelligent planning, and focused effort.” – Paul J. Meyer (source)

Duke tracks your tasks for you so your mind is free to focus on the tasks at hand. It's,

  • text-based
  • intuitive
  • quick LIGHTNING FAST in response time
  • All you need to do is,
  1. Download it from here.
  2. Double-click it.
  3. Add your tasks.
  4. Let it manage your tasks for you 😉

And it is FREE!

Features:

  • Managing tasks
  • Managing deadlines
  • Organising tasks
  • [ ]Reminders (coming soon)
    If you Java programmer, you can use it to practice Java too. Here's the main method:

public class Main { public static void main(String[] args) { new Duke(data/tasks.txt).run(); } }

Copy link

@vvidday vvidday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, your code looks good and I really like how you organized it nicely into different packages, especially commands! There's a couple of minor code quality / coding standard issues below, but other than that, LGTM!


import java.time.format.DateTimeFormatter;

public abstract class Command {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the use of an abstract class here, as well as your great organization in the commands package!

}
}

public void store(TaskList toStore) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could consider writing javadocs for these more complicated public facing methods, to give others a clearer picture of what they do?


@Override
public void execute(TaskList taskList, UI ui, Storage storage) {
String taskDesc = input.substring(5);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a named constant instead of a magic number would make the code here (and other parts) clearer?

private final int taskID;

public DeleteTaskCommand(String input) throws DukeException {
if (!checkValid(input)) throw new DukeException(" ☹ OOPS!!! Please enter task to delete.");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the conditional here be wrapped in curly brackets?

return taskList.size() == 0;
}

public int size() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this method name be a verb? e.g. getSize()

Comment on lines 29 to 30
if (isEmpty()) {
throw new DukeException(" ☹ OOPS!!! Seems like your list is empty.");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you thought of & handled this edge case!

Copy link

@marcusczh marcusczh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than some minor code standard issues, the overall code looks good and is ready to merge! I really like the use of OOP to abstract out the common functionalities as well!

@@ -0,0 +1,44 @@
package Duke.commands;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to consider changing names representing packages to all lowercase, to conform to the coding standard.

}
}

public void run() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like how all the detailed functionalities of Duke.run() are abstracted further into classes.

@@ -0,0 +1,66 @@
package Duke.storage;

import Duke.tasks.*;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to consider avoiding wildcard imports

import Duke.commands.*;
import Duke.exceptions.DukeException;

public class Parser {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how you abstracted the functionalities here into separate Command classes


public class MarkTaskCommand extends Command {

private final int taskID;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think taskID can be written as taskId instead to conform to the camelCase standard

@Isaaclhy00 Isaaclhy00 changed the title Isaac iP [Isaac] iP Sep 3, 2022
Duke uses terminal to receive inputs and display outputs. This interface is ugly and not user friendly.

New GUI was implemented to make Duke appear in chat form, much more friendly.
Undo has some bugs

It does not work as intended after some testing

The implementation was changed to fix the bugs
GUi is lacktuster

Made several improvements (added profile picture, etc...)

Fixed several bugs from previous code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants