[Cheong Yee Ming] iP #462

CheongYeeMing · 2021-08-25T21:08:33Z

Duke

"The way to get started is to quit talking and begin doing." -Walt Disney (source)

DukePro frees your mind of having to remember things you need to do. It's,

text-based
easy to learn
~~FAST~~ SUPER FAST to use

All you need to do is

download it from here.
double-click it.
add your tasks.
let it manage your tasks for you 😉

And it is FREE!

Features:

Managing tasks
Managing deadlines (coming soon)
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) {
        Application.launch(MainApp.class, args);
    }
}

pualixue

LGTM. Just some nits to fix.

pualixue · 2021-08-31T03:58:24Z

src/main/java/duke/parser/Parser.java

+    }
+
+    /**
+     *


Is the method summary missing?

pualixue · 2021-08-31T04:01:08Z

src/main/java/duke/storage/Storage.java

+     * Saves tasks in TaskList to local directory.
+     * Folder and file will be created if not already present.
+     *
+     * @param taskList


Is the description of taskList missing?

pualixue · 2021-08-31T04:06:07Z

src/main/java/duke/ui/Ui.java

+     * @param task Task to be added into task list.
+     * @param taskLeft Total number of tasks in the task list.
+     */
+    public void taskAddedMessage(Task task, int taskLeft) {


Perhaps this method name can be changed to a verb (i.e. printTaskAddedMessage)?
Similarly for some of the methods below.

pualixue · 2021-08-31T04:09:06Z

src/main/java/duke/command/AddCommand.java

+    /**
+     * Constructor for an AddCommand.
+     *
+     * @param taskList Handles all operations regarding tasks.


I like how the descriptions have the same indentation which makes it easy to read. 👍

whoisjustinngo

LGTM, just some minor cosmetic issues to consider 🥇

whoisjustinngo · 2021-08-31T08:07:38Z

src/main/java/duke/command/Command.java

+     *
+     * @throws DukeException when an error occurs.
+     */
+    public abstract void runCommand() throws DukeException;


Maybe you can consider renaming the method execute or simply run to clearly distinguish the method to run the command from the type of command (i.e. AddCommand, DeleteCommand etc.)

whoisjustinngo · 2021-08-31T08:20:46Z

src/main/java/duke/parser/Parser.java

+     * @throws DukeException If user input is invalid.
+     */
+    public Command parseUserInput(String userInput) throws DukeException {
+        String[] splitUserInput = userInput.split(" ");


Consider using a noun like userInputTokens instead of splitUserInput

whoisjustinngo · 2021-08-31T08:32:52Z

src/main/java/duke/parser/Parser.java

+        case DEADLINE:
+            try {
+                int byIndex = commandDetails.indexOf("/by") - 1 ;
+                if (byIndex <= 0) {
+                    throw new NoDateTimeException(ui);
+                }
+                String deadlineDetails = commandDetails.substring(0, byIndex);
+                if (deadlineDetails.isBlank()) {
+                    throw new NoTaskDescriptionException(ui);
+                }
+                try {
+                    LocalDateTime by = LocalDateTime.parse(commandDetails.substring(deadlineDetails.indexOf("by") + 3),
+                            DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm"));
+                    return new Deadline(deadlineDetails, false, by);
+                } catch (DateTimeParseException e) {
+                    throw new InvalidDateTimeException(ui);
+                }
+            } catch (StringIndexOutOfBoundsException e) {
+                throw new NoTaskDescriptionException(ui);
+            }
+
+
+        case EVENT:
+            try {
+                int atIndex = commandDetails.indexOf("/at") - 1 ;
+                if (atIndex <= 0) {
+                    throw new NoDateTimeException(ui);
+                }
+                String eventDetails = commandDetails.substring(0, atIndex);
+                if (eventDetails.isBlank()) {
+                    throw new NoTaskDescriptionException(ui);
+                }
+                try {
+                    LocalDateTime at = LocalDateTime.parse(commandDetails.substring(eventDetails.indexOf("at") + 3),
+                            DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm"));
+                    return new Event(eventDetails, false, at);
+                } catch (DateTimeParseException e) {
+                    throw new InvalidDateTimeException(ui);
+                }
+            } catch (StringIndexOutOfBoundsException e) {
+                throw new NoTaskDescriptionException(ui);
+            }
+


The code in the Deadline and Event cases are very similar hence I think you could combine both into one case; alternatively, assuming all taskType inputs are valid, consider the ToDo case first and let the Deadline and Event be the default. To account for the 'by' and 'at' issue, maybe you can define an auxiliary delimiter string, if event then the string is 'at', else 'by'. This might help significantly reduce code duplication 👍🏼

whoisjustinngo · 2021-08-31T08:36:45Z

src/main/java/duke/task/TaskList.java

+ */
+public class TaskList {
+
+    private ArrayList<Task> list;


Perhaps tasks would be more descriptive than list

whoisjustinngo · 2021-08-31T08:39:32Z

src/main/java/duke/parser/Parser.java

+        case "list":
+            return new ListCommand(taskList, storage, ui);
+
+        case "bye":
+            return new ExitCommand(taskList, storage, ui);
+
+        case "deadline":
+            return new AddCommand(taskList, storage, ui, parseTaskInput(TaskType.DEADLINE, userInput));
+
+        case "event":
+            return new AddCommand(taskList, storage, ui, parseTaskInput(TaskType.EVENT, userInput));
+
+        case "todo":
+            return new AddCommand(taskList, storage, ui, parseTaskInput(TaskType.TODO, userInput));
+
+        case "done":
+            return new DoneCommand(taskList, storage, ui, Integer.parseInt(splitUserInput[1]));
+
+        case "delete":
+            return new DeleteCommand(taskList, storage, ui, Integer.parseInt(splitUserInput[1]));
+
+        case "find":
+            return new FindCommand(taskList, storage, ui, userInput.substring(userInput.indexOf(" ") + 1));
+
+        default:
+            throw new NoSuchCommandException(ui);
+        }


I like how clean and neat this is 👍🏼

chengseong

LGTM! Just a few nits with the JavaDocs but generally looks great!

chengseong · 2021-09-04T04:45:13Z

src/main/java/duke/command/AddCommand.java

@@ -5,14 +5,33 @@
 import duke.task.TaskList;
 import duke.ui.Ui;

+/**
+ * Command to add a task to the task list.


I think it would be good to document down what is contained in each Command

chengseong · 2021-09-04T04:46:29Z

src/main/java/duke/command/Command.java

 public abstract class Command {
    protected final TaskList taskList;
    protected final Storage storage;
    protected final Ui ui;

+    /**
+     * Constructor for a DukeCommand.


You mention a DukeCommand in the JavaDocs, but the constructor creates a Command, so it might be good to streamline this and standardize to 1 name for the Command class

chengseong · 2021-09-04T04:48:33Z

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

+ *
+ * @author Cheong Yee Ming
+ * @version Duke A-JavaDoc
+ */
 public class Deadline extends Task {

    protected final LocalDateTime by;


A name like by is not really descriptive enough. Maybe consider something like deadline.

chengseong · 2021-09-04T04:49:22Z

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

+ *
+ * @author Cheong Yee Ming
+ * @version Duke A-JavaDoc
+ */
 public class Event extends Task {

    protected final LocalDateTime at;


Same as by, at is not very descriptive

No assertions used in code. Assertions help detect programmer mistakes. Adding assert statement to the following classes: * `AddCommand` * `DeleteCommand` * `DoneCommand` * `Parser` * `Storage` * `Deadline` * `Task` * `TaskList` Assert statements are added in cases where `null` is passed into the respective `Task` constructors, detects empty tasks.

Nested class. Outer class now had 2 responsibilities. Breaches the Single Responsibility Principle. Extracting nested class `ExitProgram` within `Main` class. Avoid nesting of classes, adhere to Single Responsibility Principle.

For loops used for iteration. Too many lines of code, does not look good. Replace for loops with streams. Benefits of streams: * Write functions at more abstract level * Make code more expressive * Less verbose code * More Intuitive code * Less error prone code

A-Assertions

A-CodeQuality

A-Streams

…ng/ip

damithc and others added 17 commits July 29, 2021 20:30

Add Gradle support

d839859

Level-1

5e92ee7

Level-2

6969459

Level-3

8972ebe

Level-4

1436982

A-TextUiTesting

4bac102

Level-5

c104015

Level-6

87e0083

Level-7

7ba16ae

Level-8

424d729

Level-8

8979148

A-MoreOOP

69ad545

A-Packages

ddb01e3

A-JUnit

15ee3f3

A-JavaDoc

19b9b20

branch-A-CodingStandard

c85d8a3

Level-9

40778c3

pualixue reviewed Aug 31, 2021

View reviewed changes

whoisjustinngo reviewed Aug 31, 2021

View reviewed changes

chengseong reviewed Sep 4, 2021

View reviewed changes

CheongYeeMing and others added 10 commits September 7, 2021 04:52

Merge remote-tracking branch 'origin/add-gradle-support'

b1349d6

A-Gradle

d2c09d3

A-CheckStyle

fdfd51d

Level-10

d1862f5

A-CodeQuality

b5e4370

Nested class. Outer class now had 2 responsibilities. Breaches the Single Responsibility Principle. Extracting nested class `ExitProgram` within `Main` class. Avoid nesting of classes, adhere to Single Responsibility Principle.

A-Streams

cc2c726

For loops used for iteration. Too many lines of code, does not look good. Replace for loops with streams. Benefits of streams: * Write functions at more abstract level * Make code more expressive * Less verbose code * More Intuitive code * Less error prone code

Merge pull request #1 from CheongYeeMing/branch-A-Assertions

78718b2

A-Assertions

Merge pull request #2 from CheongYeeMing/branch-A-CodeQuality

6afe26b

A-CodeQuality

Merge pull request #3 from CheongYeeMing/branch-A-Streams

9038a5e

A-Streams

CheongYeeMing and others added 9 commits September 11, 2021 12:59

Merge branch 'branch-A-CodeQuality' of https://github.com/CheongYeeMi…

484517f

…ng/ip

Merge branch 'branch-A-Streams' of https://github.com/CheongYeeMing/ip

838ce6b

Merge branch 'master' of https://github.com/CheongYeeMing/ip

9ea146b

BCD-Extension

3e0f472

A-BetterGui

d7a6671

A-UserGuide

e844522

A-Release

72e043b

Merge branch 'master' of https://github.com/CheongYeeMing/ip

6f6745e

A-UserGuide

3c14a89

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cheong Yee Ming] iP #462

[Cheong Yee Ming] iP #462

CheongYeeMing commented Aug 25, 2021 •

edited

Loading

pualixue left a comment

pualixue Aug 31, 2021

pualixue Aug 31, 2021

pualixue Aug 31, 2021

pualixue Aug 31, 2021

whoisjustinngo left a comment

whoisjustinngo Aug 31, 2021

whoisjustinngo Aug 31, 2021

whoisjustinngo Aug 31, 2021

whoisjustinngo Aug 31, 2021

whoisjustinngo Aug 31, 2021

chengseong left a comment

chengseong Sep 4, 2021

chengseong Sep 4, 2021

chengseong Sep 4, 2021

chengseong Sep 4, 2021

+                  }
+                  /**
+                   *

[Cheong Yee Ming] iP #462

Are you sure you want to change the base?

[Cheong Yee Ming] iP #462

Conversation

CheongYeeMing commented Aug 25, 2021 • edited Loading

Duke

pualixue 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

whoisjustinngo 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

chengseong 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

CheongYeeMing commented Aug 25, 2021 •

edited

Loading