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

[Liu Han] ip #500

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

Conversation

SweetPotato0213
Copy link

@SweetPotato0213 SweetPotato0213 commented Aug 29, 2022

Duke Helper

“Your mind is for having ideas, not holding them.” – David Allen Quoted from here

Introduction

Duke is an interactive personal chat robot designed for people tired of remembering all the tasks they've done or they're going to do. It's

  • text-based
  • easy to use
  • retentive enough to remember all the tasks you've ever mentioned
  • free!

Steps to use

  1. Download it from here
  2. Go to the directory and double-click Duke
  3. Input your tasks (including to-do, event, and deadline)
  4. Let it manage your tasks for you 😆

Features

  • Managing events
  • Managing to-do's
  • Managing dealines
  • Reminders (coming soon)

Addition

If you are a Java programmer, you can also use it to practice Java. Here's the main method:

public class Main {
    public static void main(String[] args) {
        Application.launch(MainApp.class, args);
    }
}

Copy link

@sarrrdin sarrrdin left a comment

Choose a reason for hiding this comment

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

Looks good. 👍
Some cosmetic changes are requested to follow coding standards.
LGTM.

Comment on lines 6 to 11
bye,
list,
mark,
unmark,
todo,
delete

Choose a reason for hiding this comment

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

Enum names should be written in PascalCase or all uppercase?

BYE,
LIST,
...

todo,
delete
}

public static void main(String[] args) {
String logo = " ____ _ \n"

Choose a reason for hiding this comment

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

I'm thinking that maybe variable logo can be set as final?

@@ -13,7 +13,7 @@ then
fi

# compile the code into the bin folder, terminates if error occurred
if ! javac -cp ../src/main/java -Xlint:none -d ../bin ../src/main/java/*.java
if ! javac -cp ../src/main/java -Xlint:none -d ../bin ../src/main/java/Duke.java

Choose a reason for hiding this comment

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

I like how you changed the accessibility where it is more specific 😄

public static void main(String[] args) {
String logo = " ____ _ \n"
+ "| _ \\ _ _| | _____ \n"
+ "| | | | | | | |/ / _ \\\n"
+ "| |_| | |_| | < __/\n"
+ "|____/ \\__,_|_|\\_\\___|\n";
System.out.println("Hello from\n" + logo);
String hello = "____________________________________________________________\n" +

Choose a reason for hiding this comment

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

I think can leave an empty space here?

@@ -1,10 +1,53 @@
import java.util.*;

Choose a reason for hiding this comment

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

Import of classes should be listed explicitly?

" Here are the tasks in your list:";

System.out.println(strlst);

Choose a reason for hiding this comment

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

I think can remove a blank line here 😃

Update all the codes to follow coding standards
correct the functions for level 2
Correct the functions and separate Task class from Duke class file
" ____________________________________________________________\n";
System.out.println(hello);

List<Task> list = new ArrayList<Task>();

Choose a reason for hiding this comment

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

Maybe you can change this variable name to tasks or something like that to make it clearer?


System.out.println(strlst);

String todolist = "____________________________________________________________\n" +

Choose a reason for hiding this comment

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

Should this be todoList?

Copy link

@Qiaoran-M Qiaoran-M left a comment

Choose a reason for hiding this comment

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

LGTM!

int number = sc.nextInt();
Task task = list.get(number - 1);
task.markAsDone();
System.out.println(" ____________________________________________________________\n" +

Choose a reason for hiding this comment

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

You may consider placing line break before operator +




String strlst = "____________________________________________________________\n" +

Choose a reason for hiding this comment

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

Maybe you can change this variable to stringList or something like that to make it clearer?

public class Duke {

enum Ability {

Choose a reason for hiding this comment

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

Good use of enums for different commands!

Copy link

@tensaida tensaida left a comment

Choose a reason for hiding this comment

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

You've made good progress in your iP. All the best for future weeks

int number = sc.nextInt();
Task task = list.get(number - 1);
task.markAsDone();
System.out.println(" ____________________________________________________________\n" +
Copy link

Choose a reason for hiding this comment

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

Also consider storing the separator as a constant

String hello = " ____________________________________________________________\n" +
" Hello! I'm Duke\n" +
" What can I do for you?\n" +
" ____________________________________________________________\n";
Copy link

Choose a reason for hiding this comment

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

Good minimalist design

String input = sc.next();

while (!input.equals("bye")) {
if (input.equals("list")) {
Copy link

Choose a reason for hiding this comment

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

Consider using switch case instead of if-else

Correct and supplement
Update and correct the codes
Add delete function
Save the tasks in the hard disk automatically whenever the task list changes.
Teach Duke how to understand dates and times. Store deadline dates as a java.time.LocalDate objects. Accept dates in yyyy-mm-dd format (e.g., 2019-10-15) and print in a different format MMM dd yyyy e.g., (Oct 15 2019).
Do it in a new branch
Update code in a new branch
it's for merge trial
Added Storage, Parser, TaskList classes
Package all classes into duke
SweetPotato0213 and others added 23 commits September 7, 2022 00:24
Use Gradle to automate some of the build tasks of the project. Now I can run Duke.main() using Gradle but cannot run it in the terminal (If I do java Duke.java it will report 9 errors saying cannot find symbols and compilation failed. I don't know why.).
create a folder (test/java/duke) to place all the test classes, use Assertions to compare the expected and actual outcomes
Package the app as an executable JAR file so that it can be distributed easily.
Add JavaDoc comments to the code. Add header comments to all non-private classes/methods.
Tweak the code to comply with a coding standard.
Give users a way to find a task by searching for a keyword.
Place line break before operators.
Modify parser method (return value from void to String)
Make some cosmetic changes to the codes according to the iP code quality feedback from teaching group
Add assert statement in printList method in Parser class
Improve the Javadoc statement for getStatus method
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.

5 participants