[Chu Yi-Ting] iP #426

chuyiting · 2020-08-26T09:26:59Z

No description provided.

Classes related to Duke do not meet the coding standard. A package named Duke is created to refactor the classes. Classes related to Duke should all be packaged into a single package, by doing so, classes related to Duke is encapsulated. Refer to Package tutorial on GeeksForGeeks https://www.geeksforgeeks.org/packages-in-java/

Use a csv file as database.

Define standard format for Date.

This reverts commit 1293c7a.

Scalability of the program was not good because of the weak abstraction design. Refactor i/o related classes into ui package, encapsulate data storage and decrease the dependency between each classes.

# Conflicts: # src/main/java/duke/DukeCommandMatcher.java # src/main/java/duke/tasks/SingletonTaskList.java # src/main/java/duke/utils/Constants.java # src/main/java/duke/utils/UtilFunction.java

Build Jar file

bchenghi

LGTM! Overall, I thought the coding standard was really good! There were no arrow heads and the code was easy to read. I have some minor suggestions, but other than that, I feel that it's good.

bchenghi · 2020-08-31T15:05:01Z

src/main/java/duke/DukeCommandMatcher.java

+    }
+
+
+    public String matchCommand(String command) throws CommandNotFoundException, NullCommandException,


This method might be a little long?

Yeah, it does not look nice, but because of the same behaviour that Duke would respond to all these exceptions, I decide to throw all of them and handle it together in the highest level. Thanks for the suggestion.

bchenghi · 2020-08-31T15:06:19Z

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

+     * @throws IllegalStateException
+     * @see IllegalStateException
+     */
+    public static void loop(Storage database) throws IllegalStateException {


loop as a method name, could it be a little vague?

Hmm, as I imagine a working application is something that is looping all the time until you shut it down. Any better suggestions for the name>

src/main/java/duke/utils/Constants.java

bchenghi · 2020-08-31T16:11:43Z

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

+     * Real all the tasks from the database.
+     * @return the list of all the tasks from the database
+     */
+    public List<Task> readAll() {


Method name might be a little vague? Since the method returns a list.

Hmm, yeah, I kind of follow the idea of CRUD (the read), as this method scans through the database, therefore I name it readAll. Any suggestions?

bchenghi · 2020-08-31T16:16:03Z

src/main/java/duke/utils/UtilFunction.java

+        }
+    }
+
+    private static void lineSentence(String sentence) {


Perhaps method names can be verbs or actions?

Hahaha, I spent few minutes pondering on the name of this method, as what it does is basically "auto-change-line" if the sentence is longer than the console (the same as wrapText). I did not know how to name it at that time, I guess ill follow the JavaFX convention. thanks for the suggestion

WhiteLio

looks great! keep it up :)

src/main/java/duke/DukeCommandMatcher.java

WhiteLio · 2020-09-01T09:10:05Z

src/main/java/duke/DukeCommandMatcher.java

+     * @throws CommandNotFoundException
+     * @throws NullCommandException
+     * @throws LackOfTimeException
+     * @throws NullCommandContentException
+     * @throws TaskOutOfBoundException
+     * @throws TaskNotSpecifyException
+     * @throws DateFormatException


you should specify when these exceptions are thrown!
eg @throws CommandNotFoundException If the user command is not in the command list

Thanks for the remind, I forgot to leave a readable note for them. Will add soon.

WhiteLio · 2020-09-01T09:12:32Z

src/main/java/duke/DukeCommandMatcher.java

+    }
+
+    private String handleExit() {
+        System.out.println("Farewell/再見/さようなら～～");


귀여워요 haha

Good suggestion haha

WhiteLio · 2020-09-01T09:16:44Z

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

+        switch (taskStr[0]) {
+        case "T": {
+            task = new ToDo(taskStr[2], taskStr[1]);
+            break;
+        }
+        case "E": {
+            task = new Event(taskStr[1], taskStr[2], taskStr[3]);
+            break;
+        }
+        case "D": {
+            task = new Deadline(taskStr[1], taskStr[2], taskStr[3]);
+            break;
+        }
+        default: {
+            task = null;
+            break;
+        }
+        }


those curly braces are unnecessary! i think the code would look cleaner without them~

WhiteLio · 2020-09-01T09:21:57Z

src/main/java/duke/utils/Constants.java

+    /**
+     * Exit pattern of the exit command for Duke.
+     */
+    public static final String EXITPATTERN = ("^(b|B)(y|Y)(e|E)$");


you should use underscores to separate words! also, associated constants (final variables) should be prefixed by a common type name
eg PATTERN_EXIT, PATTERN_DONE

Ahh, right, that is a part of the coding standard, I forgot about it, yeah, maybe I'll also consider changing it into an Enum class, thanks.

hyngkng

Overall I like your coding style and your code generally follows the coding standard. Just some nits to fix!

hyngkng · 2020-09-02T23:36:07Z

src/main/java/duke/tasks/Event.java

+    private String duration;
+
+    /**
+     * Default constructor for a new event object.


Shouldn't there be a space between the description and param section?

Add assertions

Start CI

Create gradle.yml

The listener was still enabled after "bye" command. The listener is disabled after "bye".

Add command instruction to README.

chuyiting added 19 commits August 19, 2020 10:16

Level1 - Echo built

fd4303b

Level2 - Add & List functionality built

3b25d64

Level3 Mark as Done built

0c56624

Level 4 - Todos, Deadlines, events built

fccb61d

Test added

ca26368

Exceptions of possible incomplete inputs

31c5471

Index out of bound customized

1530c91

Task Deletion built

4cee1d4

Level-7 Save

d7bdf71

Use a csv file as database.

Level8. Dates and Times

1293c7a

Define standard format for Date.

Revert "Level8. Dates and Times"

4fb7345

This reverts commit 1293c7a.

A-MoreOOP

bc8b238

Scalability of the program was not good because of the weak abstraction design. Refactor i/o related classes into ui package, encapsulate data storage and decrease the dependency between each classes.

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

104b141

# Conflicts: # src/main/java/duke/DukeCommandMatcher.java # src/main/java/duke/tasks/SingletonTaskList.java # src/main/java/duke/utils/Constants.java # src/main/java/duke/utils/UtilFunction.java

Add JUnit test

a3e5860

A-Jar

500f6cd

Build Jar file

Add Javadoc

086187b

Add find functionality.

3319d6e

Adjust to coding standard.

40be3fd

bchenghi reviewed Aug 31, 2020

View reviewed changes

Add checkstyle and reduce errors to 0

5dc2e7b

WhiteLio reviewed Sep 1, 2020

View reviewed changes

hyngkng reviewed Sep 3, 2020

View reviewed changes

chuyiting and others added 7 commits September 3, 2020 22:06

Build gui for Duke

f0964bc

Update GUI

5f6b4b7

Update code with assertions.

d310733

Merge pull request #1 from chuyiting/branch-assertion

f382412

Add assertions

Refactor Cli class to conform to SLAP.

1e686c6

Create gradle.yml

d4fb794

Start CI

Merge pull request #2 from chuyiting/branch-CI

bd3d73a

Create gradle.yml

chuyiting and others added 26 commits September 11, 2020 00:27

Update Windows and Linux dependency for JavaFX to pass CI.

03e8e25

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

e40b5d3

Delete IO redirection test

2d42a88

Merge branch 'branch-codeQuality'

03879be

Delete repeated yml file.

853fdab

Build natural date interpreter.

93c78a6

Fix Junit testing.

42236d8

Fix test checkStyle.

b5f3b7d

Fix GUI bugs

05a050a

Fix style problem

764be36

Fix input diable bug.

2630d40

The listener was still enabled after "bye" command. The listener is disabled after "bye".

Add event duration function.

767daf8

Refactor parser

c6a9ba1

Add tag GUI

60a1d91

Fix few bugs.

92b6ead

Fix checkstyle.

0a9ef0f

Fix checkstyle

de00106

Add error hightlighting.

8df5694

Fix checkstyle.

8a35983

Add Ui picture

d7fef5c

Set theme jekyll-theme-architect

4ab4c2a

Update User Guide

f53cafa

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

d451fbf

Fix small find bugs

83888ed

Fix minor textfield bug.

7f7a278

Add command instruction to README.

Fix css and javaFX error messages.

98bcb09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Chu Yi-Ting] iP #426

[Chu Yi-Ting] iP #426

chuyiting commented Aug 26, 2020

bchenghi left a comment

bchenghi Aug 31, 2020

chuyiting Sep 3, 2020

bchenghi Aug 31, 2020

chuyiting Sep 3, 2020

bchenghi Aug 31, 2020

chuyiting Sep 3, 2020

bchenghi Aug 31, 2020

chuyiting Sep 3, 2020

WhiteLio left a comment

WhiteLio Sep 1, 2020

chuyiting Sep 3, 2020

WhiteLio Sep 1, 2020

chuyiting Sep 3, 2020

WhiteLio Sep 1, 2020

WhiteLio Sep 1, 2020

chuyiting Sep 3, 2020

hyngkng left a comment

hyngkng Sep 2, 2020

		}


		public String matchCommand(String command) throws CommandNotFoundException, NullCommandException,

[Chu Yi-Ting] iP #426

Are you sure you want to change the base?

[Chu Yi-Ting] iP #426

Conversation

chuyiting commented Aug 26, 2020

bchenghi 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

WhiteLio 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

hyngkng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment