[Joven Heng] IP #88

Joven-Heng · 2020-08-21T16:15:49Z

No description provided.

…d test or inserted test cases, will do after level 7

Fixed a typo from Level-4

Added in the expected and input test cases.

Text file used for to remember the list of jobs done by the chatbot Let's * Use a storage class to handle the read and writing of txt files. * Use the txt file to store the current list such that it can be reused again after closing Duke

Delete task.txt

Tasks has different dates and times when they are created and due. Create DateTimes to show when a task is created and when deadlines and events are due.

# Conflicts: # src/main/java/Deadline.java # src/main/java/Duke.java # src/main/java/EventTask.java

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

Add Unittests for adding deadlines.

Users are unable to find tasks based on keywords. Implement find feature to allow users to search for tasks based on certain keywords.

# Conflicts: # src/main/java/duke/Duke.java # src/main/java/duke/parser/Parser.java # src/main/java/duke/storage/Storage.java # src/main/java/duke/task/Task.java # src/main/java/duke/task/deadline/Deadline.java # src/main/java/duke/task/eventtask/EventTask.java # src/main/java/duke/task/todo/ToDo.java # src/main/java/duke/tasklist/TaskList.java # src/main/java/duke/ui/UI.java

# Conflicts: # src/main/java/duke/ui/UI.java

Due to gradle not intepreting UTF-8 characters, change all special characters to normal ones

benclmnt

Overall, I found your code easy to read. I noted a few minor coding standard violations too and give some suggestions on a perhaps more intuitive variable name. Other than that, it looks good to me! 👍

benclmnt · 2020-08-31T10:06:44Z

src/main/java/duke/dukeexception/DukeIncorrectTimeFormatException.java

+
+public class DukeIncorrectTimeFormatException extends DukeException {
+
+    public DukeIncorrectTimeFormatException(String message){


Perhaps leave a space after the argument closing parenthesis for better readability? I noticed the same issue in several other places too...

Suggested change

public DukeIncorrectTimeFormatException(String message){

public DukeIncorrectTimeFormatException(String message) {

benclmnt · 2020-08-31T10:09:56Z

src/main/java/duke/Duke.java

+     *
+     * @param sc Scanner object used to read users inputs
+     */
+    public static void handler(Scanner sc) {


Perhaps handleUserInputs will make a better variable name here?

benclmnt · 2020-08-31T10:13:08Z

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

+            } else if (response.indexOf("deadline ") == 0) {
+                if (response.length() <= 9) {
+                    throw new EmptyDescriptionException("deadline");
+                }


Perhaps you can add a space after each if block to help increase readability.

benclmnt · 2020-08-31T10:15:38Z

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

+                indexer = Integer.parseInt(response.replaceAll("\\D+", "")) - 1;
+                ui.replyDone(indexer);
+            } else if (response.indexOf("todo ") == 0) {
+                if (response.length() <= 5) {


Can I clarify that 5 here comes from length("todo ") ? If so, perhaps it is better to use length("todo ") or a constant here instead of magic numbers?

I noticed similar issues in several other places too...

benclmnt · 2020-08-31T10:16:55Z

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

+     * @param shelf the TaskList that we are updating
+     * @throws IOException if the file cannot be found
+     */
+    public void updateFile(TaskList shelf) throws IOException {


Perhaps taskList will give a more clearer context instead of shelf?

benclmnt · 2020-08-31T10:18:46Z

src/main/java/duke/tasklist/TaskList.java

+     * @throws DukeTaskNonExistException if the index provided is out of range of the arraylist
+     */
+    public void delete(int index) throws DukeTaskNonExistException {
+        if (index < 0 | index >= shelf.size()) {


Perhaps you mean || here? I noticed the same issue in several other places too..

benclmnt · 2020-08-31T10:21:10Z

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

+     */
+    public void listen(String response) {
+        try {
+            int indexer;


Perhaps a more intuitive variable name here? If I understand correctly, it is used to determine the index of task in your tasklist, so perhaps taskIndex ?

# Conflicts: # src/main/java/duke/Duke.java

# Conflicts: # src/main/java/duke/Duke.java # src/main/java/duke/parser/Parser.java # src/main/java/duke/tasklist/TaskList.java

Joven-Heng · 2020-09-01T12:42:51Z

Overall, I found your code easy to read. I noted a few minor coding standard violations too and give some suggestions on a perhaps more intuitive variable name. Other than that, it looks good to me! 👍

Thanks so much :) Ill make the changes in the following days :D

AaronnSeah

LGTM, just a few nits to pick.

AaronnSeah · 2020-09-02T09:46:01Z

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

+    }
+
+    /**
+     * If a tasklist.txt file exists, this method would load it into the TaskList.


Perhaps it can be started with an action word e.g. returns?
What do you think about "Loads tasks in tasklist.txt file into the TaskList if the file exists"?
I have noticed the same issue in several places too.

AaronnSeah · 2020-09-02T09:47:20Z

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

+    /**
+     * If a tasklist.txt file exists, this method would load it into the TaskList.
+     *
+     * @return A TaskList that is loaded with the tasks recorded on the txt file


Perhaps it could be ended with a full stop to be consistent with the coding standards?
I have noticed the same issue in several places too.

AaronnSeah · 2020-09-02T09:53:57Z

src/main/java/duke/ui/UI.java

+    /**
+     * Method that prints the goodbye statement
+     */
+    public String replyBye() {


Perhaps the naming of the method could be improved?
The method name suggests to me that a "bye" response is printed and that it is a void return method instead of a String return method.
I have noticed the same issue in several places too.

AaronnSeah · 2020-09-02T09:56:59Z

test/DukeTest.java

+        try {
+            ui.addDeadline("finish homework!", "2020-10-02 11:44");
+            String currentTime = LocalDateTime.now().format(formatter);
+            assertEquals("Got it. I've added this task: \r\n  [D][-] finish homework! [created on " +


Perhaps the + sign can be moved to the current second line of String to be consistent with the coding standards?

Implement assert feature

# Conflicts: # src/main/java/duke/storage/Storage.java # src/main/java/duke/task/Task.java # src/main/java/duke/tasklist/TaskList.java # src/main/java/duke/ui/UI.java

Improve padding, implement color between conversations. Change font

# Conflicts: # task.txt

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

Add Gradle support

3b19ba1

Chatbot level 1 v1.0

1881a89

Level 2. Add, list Ver 1.0

9b9b673

Level-3: Mark as done Version 1.0

0f222ea

Level-3 Changed indentations and fixed method names v1.1

0e746a8

Level 4. ToDos, Events, Deadlines, v1.0

91eca28

A-TextUiTesting Edited runtest.bat v1.0, have not changed the expecte…

21cdd29

…d test or inserted test cases, will do after level 7

Level-5 Handle Errors v1.0

ba02434

Fixed a typo from Level-4

level-6 delete v1.0

5683b64

Fixed very minor typos.

6a398a3

Added in the expected and input test cases.

Implement save feature and storage class

0c99b03

Text file used for to remember the list of jobs done by the chatbot Let's * Use a storage class to handle the read and writing of txt files. * Use the txt file to store the current list such that it can be reused again after closing Duke

Delete task.txt

065beae

Delete task.txt

Delete task.txt

f274ec2

Add DateTime fields

7fbf835

Tasks has different dates and times when they are created and due. Create DateTimes to show when a task is created and when deadlines and events are due.

Merge branch 'branch-Level-7'

2210421

Merge branch 'branch-Level-8'

e1c7929

# Conflicts: # src/main/java/Deadline.java # src/main/java/Duke.java # src/main/java/EventTask.java

Organise classes into packages

fadb221

Add Duketest.java

7ec3f00

Add Unittests for adding deadlines.

Create javadoc

9870e9b

Tweak code to apply to given coding standard

0dbd7ab

Implement find feature

840bb2a

Users are unable to find tasks based on keywords. Implement find feature to allow users to search for tasks based on certain keywords.

Merge branch 'branch-A-JavaDoc'

86f09d9

Merge branch 'Branch-Level-9'

20d706d

# Conflicts: # src/main/java/duke/ui/UI.java

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

271c8c9

Add gradle build support with Junit Test functionality

80ce98a

Due to gradle not intepreting UTF-8 characters, change all special characters to normal ones

Fix minor typo for Task requiring UTF-8 encoding

d6e32f5

Create Checkstyle and clean code

a9e6f21

benclmnt reviewed Aug 31, 2020

View reviewed changes

Joven-Heng added 5 commits September 1, 2020 18:20

Merge branch 'Branch-Level-10-GUI'

61709f5

Merge branch 'Branch-A-Gradle'

39c5a7c

# Conflicts: # src/main/java/duke/Duke.java

Merge branch 'Branch-A-CheckStyle'

666d314

# Conflicts: # src/main/java/duke/Duke.java # src/main/java/duke/parser/Parser.java # src/main/java/duke/tasklist/TaskList.java

Update checkstyle rules

71cf814

Add package for gui, Fix checkstyle problems

423b984

Joven-Heng closed this Sep 1, 2020

Joven-Heng reopened this Sep 1, 2020

AaronnSeah reviewed Sep 2, 2020

View reviewed changes

Joven-Heng added 21 commits September 4, 2020 23:36

change image for GUI

a7b0641

Update Shadow Version for Fat-Jar build

5cf10fb

Implement assert feature

f2ae37c

Add Enum for commands

9c62a55

Refactor code to meet Codequality requirements

1f1847f

Merge pull request #1 from Joven-Heng/Branch-A-Assertions

4d6d834

Implement assert feature

Merge branch 'Branch-A-CodeQuality'

eb04b87

# Conflicts: # src/main/java/duke/storage/Storage.java # src/main/java/duke/task/Task.java # src/main/java/duke/tasklist/TaskList.java # src/main/java/duke/ui/UI.java

Implement Github actions

51aaad8

Create main.yml

3e97f28

Implement update feature

d8e8d3c

Tweak GUI

51b8eac

Improve padding, implement color between conversations. Change font

Fix bugs in update, change font size

4dd003c

Update README.md

c8e679e

Include UI product screenshot

732013d

Set theme jekyll-theme-time-machine

3c0bba2

Fix small typos

8a0fbda

Merge branch 'master' of https://github.com/Joven-Heng/ip

ae9d29b

Merge remote-tracking branch 'origin/branch-Level-7'

47d9ea6

# Conflicts: # task.txt

Fix file path error

3bfafba

Fix Storage issue

f6579e6

Include implementation for javafx libraries

3d0a2c8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Joven Heng] IP #88

[Joven Heng] IP #88

Joven-Heng commented Aug 21, 2020

benclmnt left a comment

benclmnt Aug 31, 2020

benclmnt Aug 31, 2020

benclmnt Aug 31, 2020

benclmnt Aug 31, 2020

benclmnt Aug 31, 2020

benclmnt Aug 31, 2020 •

edited

Loading

benclmnt Aug 31, 2020

Joven-Heng commented Sep 1, 2020

AaronnSeah left a comment

AaronnSeah Sep 2, 2020

AaronnSeah Sep 2, 2020

AaronnSeah Sep 2, 2020

AaronnSeah Sep 2, 2020


		public class DukeIncorrectTimeFormatException extends DukeException {

		public DukeIncorrectTimeFormatException(String message){

[Joven Heng] IP #88

Are you sure you want to change the base?

[Joven Heng] IP #88

Conversation

Joven-Heng commented Aug 21, 2020

benclmnt 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

benclmnt Aug 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Joven-Heng commented Sep 1, 2020

AaronnSeah 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

benclmnt Aug 31, 2020 •

edited

Loading