[Teo Zay We, Simon] iP #397

simonteozw · 2020-08-25T17:46:55Z

No description provided.

Add DateTime functionality for Events and Deadlines

Level-7: Save

Add functionality to search list based on keyWord

Add JavaDoc comments to all non-public classes and methods

Add Gradle support

Set up Checkstyle and fix styling errors

TheSpaceCuber

Great & clean code!

TheSpaceCuber · 2020-08-31T06:07:36Z

src/main/java/duke/Storage.java

+     * @param items List of tasks.
+     * @return A string that can be stored in the .txt file.
+     */
+    String allTasksCombined(ArrayList<Task> items) {


Perhaps can consider using a verb for the method name? (E.g. CombineAllTasks etc)

TheSpaceCuber · 2020-08-31T06:10:19Z

src/main/java/duke/TaskList.java

+    /**
+     * Exit function for TaskList class, writes data to .txt file.
+     */
+    public void bye() {


Bye might be a little confusing haha. Consider including the fact that it writes data.

Branch level 10

Ringo1225

The quality of your code is good, and you followed the coding standards strictly. Feel free to reply if you have any questions.

Ringo1225 · 2020-09-06T04:31:57Z

src/main/java/duke/misc/Parser.java

+     * @return A task to be added by the TaskList.
+     * @throws InvalidDescriptionException In case the task description is empty.
+     * @throws InvalidTypeException        In case the task type is not one of Event, Deadline, Todo.
+     */


The javadocs are clear and easy to understand

Ringo1225 · 2020-09-06T04:34:36Z

src/main/java/duke/misc/TaskList.java

+    /**
+     * Exit function for TaskList class, writes data to .txt file.
+     */
+    public void saveData() throws IOException {


The naming of the functions and fields are in the correct form.

Ringo1225 · 2020-09-06T04:40:22Z

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

@@ -0,0 +1,26 @@
+package duke.task;
+
+/**


The structure of your class is very clean and easy to read.

Ringo1225 · 2020-09-06T04:41:35Z

text-ui-test/input.txt

+delete -1
+delete 2
+list
+bye


You may add more test cases to test your code.

maxxyh

LGTM! Overall, really well-organised code, especially at how you handled your exceptions and the Parser. Maybe as a challenge to yourself you can consider implementing the Command Pattern and making use of regex!

maxxyh · 2020-09-06T20:42:47Z

src/main/java/duke/exception/InvalidDataException.java

+package duke.exception;
+
+/**
+ * Invalid Data Exception that extends Duke Exception class.


Perhaps can be more specific to what data/situations this exception would appear?

maxxyh · 2020-09-06T20:43:20Z

src/main/java/duke/exception/InvalidIndexException.java

+/**
+ * Invalid Index Exception that extends Duke Exception class.
+ */
+public class InvalidIndexException extends DukeException {


I really like how you are so specific with your exceptions, and how you've organised them all into an Exceptions package! Definitely learning from this!

maxxyh · 2020-09-06T20:48:43Z

src/main/java/duke/misc/Parser.java

+            String[] dl = input.split(" /by ");
+            if (dl[0].substring(8).equals("") || dl[0].substring(9).equals("")) {


Perhaps could explore regex as an extension so that your input can be slightly more flexible?

maxxyh · 2020-09-06T20:50:58Z

src/main/java/duke/misc/Storage.java

+    static final String HOME = System.getProperty("user.home");
+    static final Path DIR = Paths.get(HOME, "data");
+    static final Path PATH = Paths.get(HOME, "data", "iPStore.txt");
+    static final String IO_MESSAGE = "Sorry! There was an IOException! Initialising with an empty Tasklist!";
+    static final String INVALID_MESSAGE = "Sorry! There was an error in reading your data! "
+            +
+            "Initialising with a semi-complete Tasklist!";


I like how your constants are cleanly organised at the top!

maxxyh · 2020-09-06T21:01:17Z

src/main/java/duke/misc/TaskList.java

+/**
+ * TaskList class to store all the tasks in Duke.
+ */
+public class TaskList {


Maybe you can consider the Command pattern to handle the user commands, so some of the string output responsibilities can be abstracted out?

maxxyh · 2020-09-06T21:02:15Z

src/main/java/duke/task/DateTime.java

+/**
+ * DateTime class used to store date objects.
+ */
+public class DateTime {


I really like how you abstracted out all logic regarding dateTime formatting to this Class! It even handles valid formatting!

maxxyh · 2020-09-06T21:04:21Z

src/main/java/duke/task/Task.java

+     *
+     * @return String describing the task.
+     */
+    public String saveString() {


Perhaps a more apt method name would be getSaveString()?

maxxyh · 2020-09-06T21:05:02Z

src/test/java/duke/misc/ParserTest.java

+        try {
+            Parser.handleInput("example hello");
+        } catch (InvalidTypeException e) {
+            assertEquals("OOPS!!! I'm sorry, but I don't know what that means :-(",
+                    e.getMessage());
+        } catch (InvalidDescriptionException e) {
+            assertEquals("OOPS!!! The description of a task cannot be empty",
+                    e.getMessage());


I really like how you're also testing your exceptions! But perhaps you could also include some test cases for normal expected inputs?

Use Regex to enhance readability

Improve Coding Quality

Branch a assertions

Allow tagging when user starts word with #

Branch a better gui

damithc and others added 8 commits July 23, 2020 23:27

Add Gradle support

3b19ba1

Level 1. Greet, Echo, Exit

5367fd3

Level 2. Add, List

c024d1f

Level 3. Mark as Done

4a5832a

Level 4. ToDos, Events, Deadlines

6619055

A-TextUiTesting: Automated Text UI Testing

7e0d35e

Level 5. Handle Errors

21d7036

Level 6. Delete

1aa0735

simonteozw changed the title ~~[Teo Zay We, Simon] IP~~ [Teo Zay We, Simon] iP Aug 25, 2020

simonteozw and others added 5 commits August 26, 2020 17:51

Level-7: Save

82bb0f0

Add DateTime functionality for Events and Deadlines

b31892e

Merge pull request #1 from simonteozw/branch-Level-8

1cefc6c

Add DateTime functionality for Events and Deadlines

Merge pull request #2 from simonteozw/branch-Level-7

15b765c

Level-7: Save

Minor fix for type errors when level 7 and level 8 were merged in

3e84087

simonteozw force-pushed the master branch from 767c9b4 to 3e84087 Compare August 26, 2020 11:25

simonteozw and others added 8 commits August 26, 2020 20:48

Add better abstraction and handle more edge cases

8abfed6

Organize classes into Packages

89784c3

Add jUnit testing

dfa7ebf

Create JAR file

0118b33

Add functionality to search list based on keyWord

f093953

Merge pull request #3 from simonteozw/branch-Level-9

4706f65

Add functionality to search list based on keyWord

Add JavaDoc comments to all non-public classes and methods

a21f00e

Merge pull request #4 from simonteozw/branch-A-JavaDoc

f936755

Add JavaDoc comments to all non-public classes and methods

simonteozw force-pushed the master branch from ec85fde to 22b3915 Compare August 30, 2020 12:28

Add proper coding standards

b5532c3

simonteozw force-pushed the master branch from 22b3915 to b5532c3 Compare August 30, 2020 12:40

simonteozw and others added 3 commits August 30, 2020 20:56

Merge pull request #6 from simonteozw/add-gradle-support

fbddb68

Add Gradle support

Set up Checkstyle and fix styling errors

29222d1

Merge pull request #7 from simonteozw/branch-A-CheckStyle

c202cf6

Set up Checkstyle and fix styling errors

TheSpaceCuber reviewed Aug 31, 2020

View reviewed changes

simonteozw and others added 3 commits August 31, 2020 18:43

Merge pull request #8 from simonteozw/branch-Level-10

df51a16

Branch level 10

Improve function names

6dd68d1

Standardise error handling

4e3158d

simonteozw force-pushed the master branch 2 times, most recently from 13091fe to 1a9bdc1 Compare September 4, 2020 10:13

Ringo1225 reviewed Sep 6, 2020

View reviewed changes

maxxyh reviewed Sep 6, 2020

View reviewed changes

Enhance package structure

a6d5eb7

simonteozw force-pushed the master branch from 1a9bdc1 to a6d5eb7 Compare September 7, 2020 06:39

simonteozw and others added 13 commits September 7, 2020 15:37

Add assertions in important parts of code

1a81ce8

Use Regex to enhance readability

1445ac1

Merge pull request #9 from simonteozw/branch-A-CodeQuality

f42de19

Use Regex to enhance readability

Merge pull request #10 from simonteozw/master

8cde1a7

Improve Coding Quality

Merge pull request #11 from simonteozw/branch-A-Assertions

9bfd893

Branch a assertions

Add gradle.yml to set up CI

5a92b10

Enable tagging when user starts a word with #

9b49cd5

Merge pull request #12 from simonteozw/branch-C-Tagging

e1a5c75

Allow tagging when user starts word with #

Change duke and user images

6fe0aa4

Add constants to improve abstraction and change user input requirements

f33f801

Improve UI

44d5f7d

Merge pull request #13 from simonteozw/branch-A-BetterGui

67d61c6

Branch a better gui

Add Documentation for User Guide

a486e57

simonteozw force-pushed the master branch 6 times, most recently from 39db5ba to 61c8b84 Compare September 18, 2020 14:00

Improve exception handling and code quality

dfb9b52

simonteozw force-pushed the master branch from 61c8b84 to dfb9b52 Compare September 18, 2020 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Teo Zay We, Simon] iP #397

[Teo Zay We, Simon] iP #397

simonteozw commented Aug 25, 2020

TheSpaceCuber left a comment

TheSpaceCuber Aug 31, 2020

TheSpaceCuber Aug 31, 2020

Ringo1225 left a comment

Ringo1225 Sep 6, 2020

Ringo1225 Sep 6, 2020

Ringo1225 Sep 6, 2020

Ringo1225 Sep 6, 2020

maxxyh left a comment

maxxyh Sep 6, 2020

maxxyh Sep 6, 2020

maxxyh Sep 6, 2020

maxxyh Sep 6, 2020

maxxyh Sep 6, 2020

maxxyh Sep 6, 2020

maxxyh Sep 6, 2020

maxxyh Sep 6, 2020

		String[] dl = input.split(" /by ");
		if (dl[0].substring(8).equals("") \|\| dl[0].substring(9).equals("")) {

[Teo Zay We, Simon] iP #397

Are you sure you want to change the base?

[Teo Zay We, Simon] iP #397

Conversation

simonteozw commented Aug 25, 2020

TheSpaceCuber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ringo1225 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

maxxyh 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment