[Wesley Teo] iP #214

Bisceto · 2023-01-25T03:23:44Z

Duke Chatbot

It is only when one goes through CS2103T, does a man truly know the value of time.

With Duke Chatbot, you'll be able to easily track your tasks and events! Have more time for yourself and for CS2106!

Duke is:

A text-based management system
Easy to learn
Intuitive to use

To get started:

Download the latest release from here.
Navigate to the folder using a terminal
Run java -jar Duke.jar (Ensure you have java installed on your computer!)
Get crazy and get productive! 🥇

Features:

[x]Managing tasks
[x]Managing deadlines
[ ]Reminders
If you are a Java programmer, you can use it to practice Java too. Here's the main method:

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

…ine.java. Separated queries in to Query.java

… tasks or invalid commands

SeanChinJunKai

LGTM. Just some nits to fix.

SeanChinJunKai · 2023-01-31T13:29:56Z

src/main/java/duke/Storage.java

+
+import duke.task.*;
+
+import java.io.*;


Please list imported classes explicitly.

SeanChinJunKai · 2023-01-31T13:31:27Z

src/main/java/duke/Duke.java

+
+    public void run() {
+        ui.showWelcomeMessage();
+        boolean isExit = false;


The boolean variable isExit explains the named entity to the reader accurately and at a sufficient level of detail.

SeanChinJunKai · 2023-01-31T13:37:32Z

src/main/java/duke/task/Event.java

+    protected LocalDateTime startTime;
+    protected LocalDateTime endTime;
+    protected DateTimeFormatter inputFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd HHmm");
+    protected DateTimeFormatter displayFormatter = DateTimeFormatter.ofPattern("MMM dd yyyy HHmm");


Would it be good to leave an empty line between the variables and methods?

SeanChinJunKai · 2023-01-31T13:44:01Z

src/main/java/duke/task/Todo.java

+
+    public Todo(String input, boolean isDone) {
+        super(input, isDone);
+        this.symbol = 'T';


Would it be better to include the symbol 'T' inside the super()?

SeanChinJunKai · 2023-01-31T13:48:32Z

src/main/java/duke/Query.java

+            return QueryType.exit;
+        }
+        String[] inputArr = input.split(" ");
+        if (inputArr.length == 2 && isNumeric(inputArr[1])) {


Job well done on avoiding arrowhead style of code.

nus-jeff

Overall really good code, super easy to follow and understand 👍
Only minor formatting may be needed and your code becomes even better than before!

nus-jeff · 2023-02-02T17:54:20Z

src/main/java/duke/Parser.java

+        case list:
+            return new ListCommand();
+        case todo:
+            Todo todo = new Todo(fullCommand);
+            return new AddCommand(todo);
+        case deadline:
+            Deadline deadline = new Deadline(fullCommand);
+            return new AddCommand(deadline);
+        case event:
+            Event event = new Event(fullCommand);
+            return new AddCommand(event);
+        case mark:
+            return new MarkCommand(Integer.parseInt(fullCommandArr[1]));
+        case unmark:
+            return new UnmarkCommand(Integer.parseInt(fullCommandArr[1]));
+        case delete:
+            return new DeleteCommand(Integer.parseInt(fullCommandArr[1]));
+        case exit:
+            return new ExitCommand();


Maybe you would like to add a break statement or fallthrough comment after each case to improve code reading?

nus-jeff · 2023-02-02T17:57:22Z

src/main/java/duke/Query.java

+    }
+
+    static boolean isNumeric(String str) {
+        char[] charArray = str.toCharArray();


Nice that you placed the array sign with the variable type 👍 , perhaps you could use a plural variable name here to represent collection of chars?

nus-jeff · 2023-02-02T17:58:39Z

src/main/java/duke/Query.java

+        } else if (input.equals("bye")) {
+            return QueryType.exit;
+        }
+        String[] inputArr = input.split(" ");


Perhaps you could use a plural variable name here to represent collection of String?

nus-jeff · 2023-02-02T18:03:18Z

src/main/java/duke/Storage.java

+public class Storage {
+    protected File file;
+
+    public Storage(String fileInput) throws DukeException{


Probably you have missed this out, but maybe you would want to have a space between DukeException and the bracket.

Suggested change

public Storage(String fileInput) throws DukeException{

public Storage(String fileInput) throws DukeException {

nus-jeff · 2023-02-02T18:05:08Z

src/main/java/duke/Storage.java

+                Files.createDirectories(Paths.get(file.getParent()));
+                this.file = Files.createFile(Path.of(fileInput)).toFile();
+            }
+        } catch (IOException err){


Probably you have missed this out, but maybe you would want to have a space between the catch exception and the bracket.

Suggested change

} catch (IOException err){

} catch (IOException err) {

nus-jeff · 2023-02-02T18:06:35Z

src/main/java/duke/command/DeleteCommand.java

+import duke.task.TaskList;
+
+public class DeleteCommand extends Command {
+    private final int index;


Maybe you would like to use full caps to represent index, since its a constant?

pangrwa

Overall, your code is great. Generally follows the SLAP principle except for some of the tasks files. Some methods have arrow-head style code too!

pangrwa · 2023-02-09T01:03:50Z

src/main/java/duke/Parser.java

+        switch (inputType) {
+        case list:
+            return new ListCommand();
+        case todo:
+            Todo todo = new Todo(fullCommand);
+            return new AddCommand(todo);
+        case deadline:
+            Deadline deadline = new Deadline(fullCommand);
+            return new AddCommand(deadline);
+        case event:
+            Event event = new Event(fullCommand);
+            return new AddCommand(event);
+        case find:
+            return new FindCommand(fullCommandArr[1]);
+        case mark:
+            return new MarkCommand(Integer.parseInt(fullCommandArr[1]));
+        case unmark:
+            return new UnmarkCommand(Integer.parseInt(fullCommandArr[1]));
+        case delete:
+            return new DeleteCommand(Integer.parseInt(fullCommandArr[1]));
+        case exit:
+            return new ExitCommand();
+        default:
+            return new InvalidCommand();


Neat! An example of good code quality!

pangrwa · 2023-02-09T01:07:32Z

src/main/java/duke/command/FindCommand.java

+    public void execute(TaskList taskList, Ui ui, Storage storage) throws DukeException {
+        ArrayList<Task> foundTasks = taskList.findTasks(this.InputToSearch);
+        if (foundTasks.size() == 0) {
+            ui.showMessage("We could not find any matching tasks.");
+        } else {
+            ui.showMessage("Here are the matching tasks in your list: ");
+            for (Task foundTask : foundTasks) {
+                if (foundTask instanceof Todo) {
+                    ui.showMessage(foundTasks.indexOf(foundTask) + 1 + ". [" + foundTask.getSymbol() + "] "
+                            + "[" + foundTask.getStatusIcon() + "] " + foundTask.getDescription());
+                } else {
+                    ui.showMessage(foundTasks.indexOf(foundTask) + 1 + ". [" + foundTask.getSymbol() + "] "
+                                  + "[" + foundTask.getStatusIcon() + "] " + foundTask.getDescription()
+                                  + " (" + foundTask.getDuedateString() + ")");
+                }
+            }
+        }
+    }


Seems like an arrow-head style code, maybe perhaps the method is too long and too deeply nested. Maybe try abstracting some of the code into another method?

pangrwa · 2023-02-09T01:09:05Z

src/main/java/duke/command/ListCommand.java

+    public void execute(TaskList taskList, Ui ui, Storage storage) {
+        if (taskList.getNumTasks() == 0) {
+            ui.showMessage("You have no tasks due!");
+        } else {
+            ui.showMessage("Here are the tasks you have due!");
+            for (Task cur : taskList.getTasks()) {
+                if (cur instanceof Todo) {
+                    ui.showMessage(taskList.getTasks().indexOf(cur) + 1 + ". [" + cur.getSymbol() + "] "
+                                    + "[" + cur.getStatusIcon() + "] " + cur.getDescription());
+                } else {
+                    ui.showMessage(taskList.getTasks().indexOf(cur) + 1 + ". [" + cur.getSymbol() + "] "
+                                        + "[" + cur.getStatusIcon() + "] " + cur.getDescription()
+                                        + " (" + cur.getDuedateString() + ")");
+                }
+            }
+        }
+    }


This method is also quite long and deeply nested! Try abstracting the code out!

pangrwa · 2023-02-09T01:10:13Z

src/main/java/duke/task/Deadline.java

+    public Deadline(String input, boolean isDone) {
+        super(input, isDone);
+        this.symbol = 'D';
+        String[] temp = input.split(",");
+        this.description = temp[0];
+        LocalDate inputFormatter = LocalDate.parse(temp[1]);
+        this.duedate = inputFormatter;
+        this.duedateString = inputFormatter.format(DateTimeFormatter.ofLocalizedDate(FormatStyle.LONG));


Seems like it's not following SLAP, but I personally don't think it's a big issue here.

pangrwa · 2023-02-09T01:14:24Z

src/main/java/duke/task/Deadline.java

+    public Deadline(String input) throws DukeException {
+        super(input);
+        this.symbol = 'D';
+        String[] inputArr = input.split(" ", 2); //split 'deadline' from task input
+        if (inputArr.length == 1 || inputArr[1].isBlank()) {
+            throw new DukeException("Sorry, the description of a deadline cannot be empty!");
+        }
+        String[] descriptionArr = inputArr[1].split("/"); //split task from due date
+        if (descriptionArr.length == 1 || descriptionArr[0].isEmpty()) {
+            throw new DukeException("Please include a description or a deadline in the following format: '/yyyy-MM-dd'");
+        }
+        this.description = descriptionArr[0];
+        LocalDate inputFormatter = Parser.parseDate(descriptionArr[1]);
+        this.duedate = inputFormatter;
+        this.duedateString = inputFormatter.format(DateTimeFormatter.ofLocalizedDate(FormatStyle.LONG));
+    }


Doesn't seem like it's following SLAP too, but I personally don't think it's a big issue too.

Assertions should be used liberally for testing to spot potential bugs quickly Let's add assertions, * To Duke.java to ensure that file text is loaded for loading and savings tasks. * To AddCommand.java to ensure that task is successfully in task list * To DeleteCommand.java to ensure that task is successfully removed from task list * To TaskList.java to ensure that commands are successfully marked and unmarked.

No assertions are utilised in the codebase.

Let's rectify these comments by updating their methods. Let's remove the QueryType class to fit into the Query class

Some Command methods were unnecessarily long and cluttered.

Find functionality is case sensitive, and only matches to tasks description. A case insensitive find is more user-friendly because users cannot be expected to remember exact case of keywords. A find feature that matches dates is more user-friendly for users to see what events and deadlines they have on that day. Let's, * update the search algorithm to use case-insensitive matching * update the search algorithm to match dates in the form <yyyy-MM-dd> or <MMM dd yyyy>

Find command: make matching case insensitive and matches to dates

Find Command contains bugs when keying in dates of different formats. Let's, * Update the find command so that it can take in dates <yyyy-MM-dd> or <MMM-dd-yyyy>

Added user guide for Duke chatbot, mainly the list of features that can be used as well as their formats.

…is out of bounds. Let's, Fix the error handling for above methods to accommodate out of bounds input.

damithc and others added 14 commits July 31, 2022 17:20

Add Gradle support

556af3f

Added inputs for Duke and made Duke repeat the input.

3351cbb

Added list functionality and adding items to list

b604887

Added mark feature to mark tasks as done or undone

0e603c4

Added categories feature to tasks. Added Event.java, Todo.java, Deadl…

e624d8f

…ine.java. Separated queries in to Query.java

Added automated text UI testing

e804e9b

Added error handling features when there is a missing description for…

2377f3d

… tasks or invalid commands

Added delete task feature

7af91d6

Added save functionality to duke.txt

9b74a82

Added functionality to understand dates and times.

6ce5e1b

Refactored code to include more OOP

5dc1a16

Organised classes into packages

9d5b662

Merge branch 'add-gradle-support' of https://github.com/Bisceto/ip

34c9a62

Added gradle support

1b1e716

SeanChinJunKai reviewed Jan 31, 2023

View reviewed changes

nus-jeff reviewed Feb 2, 2023

View reviewed changes

Bisceto added 10 commits February 3, 2023 13:19

Added JUnit test cases for testing.

4a33ae7

Added JavaDoc comments to public methods

1b438fa

Merge branch 'branch-A-JavaDoc'

0ed8065

Edited code to follow coding standard

3980dca

Merge branch 'branch-A-JavaDoc'

bf48e71

Edited code to follow coding standards.

8756215

Merge branch 'branch-A-CodingStandard'

9752280

Worked on Find Command

7f65d2c

Added Find Tasks functionality

423beaf

Added Launcher.java

d4e631c

pangrwa reviewed Feb 9, 2023

View reviewed changes

Bisceto and others added 3 commits February 10, 2023 00:44

Added a GUI via JavaFX for Duke

6cf9e0d

Merge pull request #2 from Bisceto/branch-A-Assertions

895537e

No assertions are utilised in the codebase.

Bisceto and others added 11 commits February 15, 2023 11:30

Some Command methods were unnecessarily long and cluttered.

58b0723

Let's rectify these comments by updating their methods. Let's remove the QueryType class to fit into the Query class

Merge pull request #3 from Bisceto/branch-A-CodeQuality

86fe528

Some Command methods were unnecessarily long and cluttered.

Merge pull request #4 from Bisceto/branch-B-BetterSearch

44291f6

Find command: make matching case insensitive and matches to dates

Find Command: Bug fixes with regards to matching dates

fccabe6

Find Command contains bugs when keying in dates of different formats. Let's, * Update the find command so that it can take in dates <yyyy-MM-dd> or <MMM-dd-yyyy>

Update README.md

a8a7b20

Added user guide for Duke chatbot, mainly the list of features that can be used as well as their formats.

Added screenshot of application to showcase its UI

32abb59

Changed Screenshot of UI to a png file

57af595

no message

91c7435

Update README.md

deb5638

Mark, List, Delete commands not handling exceptions when index given …

7665c4f

…is out of bounds. Let's, Fix the error handling for above methods to accommodate out of bounds input.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Wesley Teo] iP #214

[Wesley Teo] iP #214

Bisceto commented Jan 25, 2023 •

edited

SeanChinJunKai left a comment

SeanChinJunKai Jan 31, 2023

SeanChinJunKai Jan 31, 2023

SeanChinJunKai Jan 31, 2023

SeanChinJunKai Jan 31, 2023

SeanChinJunKai Jan 31, 2023

nus-jeff left a comment

nus-jeff Feb 2, 2023

nus-jeff Feb 2, 2023

nus-jeff Feb 2, 2023

nus-jeff Feb 2, 2023

nus-jeff Feb 2, 2023

nus-jeff Feb 2, 2023

pangrwa left a comment

pangrwa Feb 9, 2023

pangrwa Feb 9, 2023

pangrwa Feb 9, 2023

pangrwa Feb 9, 2023

pangrwa Feb 9, 2023

	public Storage(String fileInput) throws DukeException{
	public Storage(String fileInput) throws DukeException {


		import duke.task.*;

		import java.io.*;

[Wesley Teo] iP #214

Are you sure you want to change the base?

[Wesley Teo] iP #214

Conversation

Bisceto commented Jan 25, 2023 • edited

Duke Chatbot

SeanChinJunKai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nus-jeff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pangrwa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bisceto commented Jan 25, 2023 •

edited