[Sherwin Poh Kai Xun] iP by sherrpass · Pull Request #470 · nus-cs2103-AY2122S1/ip

sherrpass · 2021-08-26T11:01:27Z

DukeMaster

“You miss 100% of the shots you don’t take.” - Kobe Bryant

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

Amazing
Fast
Convenient

All you need to do is to is,

Download it right 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)
Be actually useful (coming I hope)

Here's the main method that sparks the program off!

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

…y them back to the user when requested

Ui: deals with interactions with the user Storage: deals with loading tasks from the file and saving tasks in the file Parser: deals with making sense of the user command TaskList: contains the task list e.g., it has operations to add/delete tasks in the list Command: contains all commands

kevinchua6

Good job overall!

kevinchua6 · 2021-09-01T04:58:58Z

+     * @param data_folder_path Path to the data folder.
+     * @param data_file_path Path to the data file.
+     */
+    public Storage(String data_folder_path, String data_file_path) {


Should variables be in camelCase?

kevinchua6 · 2021-09-01T05:04:17Z

+import static org.junit.jupiter.api.Assertions.fail;
+
+public class StorageTest {
+    private final static String TEST_DATA_FOLDER_PATH = "./dataTest";


I think you shouldn't use private final static, but private static final: https://google.github.io/styleguide/javaguide.html

kevinchua6 · 2021-09-01T05:07:17Z

+        return (isDone ? "X" : " "); // mark done task with X
+    }
+
+    public boolean getDoneStatus() {


Shouldn't boolean methods sound like booleans? Any reason why you didn't do something like isDone or hasCompleted?

kevinchua6 · 2021-09-01T05:07:55Z

+        return "|" + getStatusIcon() + "|" + description;
+    }
+
+    public boolean matches(String query) {


Again, shouldn't boolean methods should sound like booleans? Perhaps a more intuitive or explanatory method name here?

kevinchua6 · 2021-09-01T05:10:57Z

@@ -0,0 +1,7 @@
+package duke;
+
+import static org.junit.jupiter.api.Assertions.*;


Should we use individual imports instead of wildcard imports?

kevinchua6 · 2021-09-01T05:27:14Z

+        ArrayList<Task> matchingTasks = tasks.getMatchingTasks(query);
+        showTasks(matchingTasks);
+    }
+}


I like that you have a consistent naming system for methods!

kevinchua6 · 2021-09-01T05:27:47Z

+        matchingTasks.removeIf(task -> !task.matches(query));
+        return matchingTasks;
+    }
+}


I like that you use verbs for all your method names!

wlren

LGTM! Looks very clean and has good quality code overall, besides the comments that the previous reviewer gave, I identified a few small details to take note of, good job!

wlren · 2021-09-02T09:07:33Z

+            case "todo":
+                newTask = new Todo(description);
+                break;
+            case "deadline":
+                newTask = new Deadline(description, date);
+                break;
+            case "event":
+                newTask = new Event(description, date);
+                break;
+            default:
+                throw new DukeException("Invalid task type provided!");


Coding standard for CS2103 requires switch statements to not have tab for case, take note to edit the default IDE settings!

wlren · 2021-09-02T09:08:38Z

+ * Represents a deadline task.
+ */
+public class Deadline extends Task {
+    protected LocalDate by;


Should perhaps be private since there is already a getter method that exposes this variable

wlren · 2021-09-02T09:09:38Z

+    @Override
+    public boolean matches(String query) {
+        return super.matches(query)
+                || by.format(DateTimeFormatter.ofPattern("MMM dd yyyy")).toLowerCase().contains(query.toLowerCase())
+                || query.equalsIgnoreCase("deadline");
+    }


A comment to explain this logic would be great

wlren · 2021-09-02T09:10:43Z

+     * @param ui UI component.
+     */
+    @Override
+    public void execute(TaskList tasks, Storage storage, Ui ui) {


Suggested change

public void execute(TaskList tasks, Storage storage, Ui ui) {

public void execute(TaskList tasks, Storage storage, Ui ui) throws DukeException {

wlren · 2021-09-02T09:11:15Z

+    private Storage storage;
+    private TaskList tasks;
+    private Ui ui;


Can be private final

wlren · 2021-09-02T09:15:05Z

+                throw new DukeException("I'm sorry, but I don't know what that means :-(");
+            }
+        } catch (DateTimeParseException e) {
+            throw new DukeException("Date provided should be in YYYY-MM-DD format.");


Is there perhaps a better way to do the error handling?

Current Duke codebase has many implicit assumptions about the Internal Invariants and Control-Flow Invariants. Unverified assumptions may cause bugs to be undiscovered during development and cause issues for the user in production. Let's add assertions throughout the codebase to verify these assumptions. Thorough checking of assumptions using assertions can ensure that bugs in the code are discovered quickly.

Current code has areas of improvement with regards to readability and code duplication. Poor readability would make the code less maintainable and comprehensable to others. Code duplication may lead to greater effort to adjust duplicated code in future. Implemenented SLAP, abstracted more logic to make code at each level simpler, removed duplicated code. SLAP, abstractions and DRY principle makes code more readable and improves code quality.

Add Assert feature

Improve code quality

Implement undo command

…a file

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

Add Gradle support

d839859

Implemented skeletal functionality to Duke, echos all, exits with bye

784ada6

Add the ability to store whatever text entered by the user and displa…

3099a61

…y them back to the user when requested

Add the ability to mark tasks as done.

8225235

Add support for tracking three types of tasks: Todo, Event and Deadline

e4e2572

semi-automate the testing of Duke

dd8baab

Add errors such as incorrect inputs entered by the user

406d820

Add Delete functionality

7fb4963

Tidy up code

9ff00d5

Add Save to and Load from capabilities from hard disk

10fca14

Add dates for deadlines and events, add new occuring function

d5c5c83

Merge branch 'branch-Level-7'

188b6ab

Merge branch 'branch-Level-8'

2ed2893

Shift all files into Duke package

f8a5b52

Add Junit Test for Storage and Deadline classes

1d97570

Add Javadoc

c730033

Fix styling and naming to fit coding standards

d1ad490

Add functionality to find tasks by queries.

edebd71

Merge branch 'branch-A-JavaDoc'

4feab0a

Merge branch 'branch-A-CodingStandard'

a98da27

Merge branch 'branch-Level-9'

c0ab76b

kevinchua6 reviewed Sep 1, 2021

View reviewed changes

wlren reviewed Sep 2, 2021

View reviewed changes

sherrpass added 6 commits September 2, 2021 23:14

Merge remote-tracking branch 'origin/add-gradle-support' into A-Gradle

151e8d7

Add gradle

a5257cb

Merge branch 'A-Gradle'

1ddb630

Add Checkstyle to detect style violations, clean up violations

54bd1f7

Add GUI using JavaFC

91f63af

Merge branch 'Level-10'

36ac23c

sherrpass and others added 17 commits September 9, 2021 23:53

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

18b8e5b

Add Assert feature

Merge branch 'master' into branch-A-CodeQuality

3e1f1f7

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

66de513

Improve code quality

Implement undo command

525fa21

Merge pull request #4 from sherrpass/branch-BCD-Extension

f56aba8

Implement undo command

Support undoing multiple commands.

0be78e1

Add extra exception handling, change profile pictures, add ui screenshot

e686719

Set theme jekyll-theme-slate

af7c1a5

Update user guide in README.md and renamed Duke class to DukeMaster

6abe1b1

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

ba5770c

Fix header issue in user guide

e74aa7c

Add response message to list command if task list is empty

5dec90c

Add new DukeDataException subclass and error handling for invalid dat…

9a2c9eb

…a file

Add undo to user guide.

b1ec315

Clean up user guide.

6ae3eb6

		@@ -0,0 +1,7 @@
		package duke;

		import static org.junit.jupiter.api.Assertions.*;

	public void execute(TaskList tasks, Storage storage, Ui ui) {
	public void execute(TaskList tasks, Storage storage, Ui ui) throws DukeException {

Uh oh!

Conversation

sherrpass commented Aug 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DukeMaster

Uh oh!

kevinchua6 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinchua6 Sep 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinchua6 Sep 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wlren left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sherrpass commented Aug 26, 2021 •

edited

Loading

kevinchua6 Sep 1, 2021 •

edited

Loading

kevinchua6 Sep 1, 2021 •

edited

Loading