[Wu Yujin] iP #155

SherryWu178 · 2020-08-22T12:01:42Z

No description provided.

* branch-Level-7: Add local storage for the tasks

* branch-Level-8: Change time field in Deadline and Event class to a LocalDate object Add content for data.txt Add data.txt # Conflicts: # src/main/java/Deadline.java # src/main/java/Duke.java # src/main/java/Event.java # src/main/java/Todo.java

* branch-Level-9: Add find function

* branch-A-JavaDo: Add Javadoc for some methods # Conflicts: # src/main/java/tasks/Task.java

* A-CodingStandard: Fix syntax error # Conflicts: # src/main/java/parser/Parser.java # src/main/java/ui/Ui.java

erisjacey

Hi! I really like the fact that you have organised your code neatly into their respective packages.

That being said, I hope my suggestions (mostly touching on coding style) will be helpful to you 😄

erisjacey · 2020-08-31T11:32:28Z

src/main/java/Duke.java

+    }
+
+    /**
+     * Run a Duke object while isExit is not changed to true


Shouldn't it be Runs instead of Run?

erisjacey · 2020-08-31T11:34:08Z

src/main/java/commands/AddCommand.java

+import main.java.tasklist.*;
+import main.java.ui.*;
+import main.java.storage.*;
+import main.java.tasks.*;


Perhaps you could import each class that you would require explicitly instead of using wildcard imports?

erisjacey · 2020-08-31T11:35:25Z

src/main/java/commands/AddCommand.java

+            return (type.equals(command.type))&&(description.equals(command.description))
+                    &&(date == date||date.equals(command.date));


Perhaps there could be spaces between each expression and its operator?

For example, line 43 could be changed to:

return (type.equals(command.type)) && (description.equals(command.description))

erisjacey · 2020-08-31T11:36:17Z

src/main/java/commands/Command.java

+import main.java.ui.Ui;
+
+/**
+ * A command object containing information parsed from an Ui object


Perhaps this line could start with * Contains information ...?

erisjacey · 2020-08-31T11:37:17Z

src/main/java/commands/DeleteCommand.java

+import main.java.tasks.*;
+import main.java.tasklist.*;


Perhaps you could import each class that you would require explicitly instead of using wildcard imports?

erisjacey · 2020-08-31T11:45:04Z

src/main/java/tasks/Event.java

+    }
+
+    @Override
+    public String data() {


Perhaps this could be renamed to getData()?

erisjacey · 2020-08-31T11:45:25Z

src/main/java/tasks/Task.java

+    protected boolean hasTime;
+    protected static int numberOfTasks = 0;
+
+


Maybe this single line break could be removed?

erisjacey · 2020-08-31T11:45:54Z

src/main/java/tasks/Task.java

+     * Reduce the count of tasks
+     */
+    public static void reduceOneTasks() {
+        numberOfTasks--;
+    }
+
+    /**
+     * Get the tick or cross sign which indicates is a task is complete
+     * @return
+     */
+    public String getStatusIcon() {
+        return (isDone ? "\u2713" : "\u2718"); //return tick or X symbols
+    }
+
+    public boolean getHasTime(){
+        return hasTime;
+    }
+
+    public LocalDate getTime(){
+        return null;
+    }
+
+    public String getDescription(){
+        return description;
+    }
+
+
+    /**
+     * mark a task as finished
+     * @return
+     */
+    public Task markAsDone() {
+        this.isDone = true;
+        return this;
+    }
+
+    @Override
+    public String toString() {
+        return "[" + getStatusIcon() + "] " + description;
+    }
+
+    /**
+     * Return the message when a task is marked as done
+     * @return mark-as-done Message
+     */
+    public String markAsDoneMessage() {
+        return "Nice!(^∇^) I've marked this task as done:\n"
+                + this.toString().replace("\u2718", "\u2713");
+    }
+
+    /**
+     * Return the message when a new task is added
+     */
+    public String addMessage() {
+        return "Got it.(^∇^) I've added this task:\n"
+                + this.toString();
+    }
+
+    /**
+     * Return the message when a task is deleted
+     */
+    public String deleteMessage() {
+        return "Got it.(^∇^) I've deleted this task:\n"
+                + this.toString();
+    }
+
+    /**
+     * Return info about the task in the format for data storage


I noticed a similar issue wrt Javadoc comment leading words in several other places.

erisjacey · 2020-08-31T11:46:37Z

src/main/java/ui/Ui.java

+     * Show all the tasks that are still in the list
+     * @param taskList
+     */
+
+    public void showTask(TaskList taskList) {
+        if (taskList.getTasks().size() == 0) {
+            System.out.println("This is no task in your task list yet. Add one now! (/^▽^)/");
+        }
+
+        int no = 1;
+        for (Task task : taskList.getTasks()) {
+            String prefix = task.toString().substring(0, 7);
+            String end = task.toString().substring(7);
+            System.out.println(prefix + no + ". " + end);
+            no++;
+        }
+    }
+
+    /**
+     * Show the tasks on certain day


I noticed a similar issue elsewhere regarding Javadoc comments.

erisjacey · 2020-08-31T11:47:01Z

src/test/java/parser/ParserTest.java

@@ -0,0 +1,34 @@
+package test.java.parser;
+
+import main.java.commands.*;


I noticed a similar issue with import wildcards in other places.

zhizhizhizhizhi

Good job overall! Can take note of the common styling issues such javadocs, whitespace before blocks and wildcard imports.

zhizhizhizhizhi · 2020-09-01T12:40:59Z

src/main/java/commands/AddCommand.java

 import main.java.tasklist.*;
 import main.java.ui.*;
 import main.java.storage.*;


Try not to use .* imports.

zhizhizhizhizhi · 2020-09-01T12:46:30Z

src/main/java/storage/Storage.java

                    String[] info = line.split(" \\| ");
                    String type = info[0];
                    int complete = Integer.parseInt(info[1]);
                    String title = info[2];
+
                    switch(type) {


Perhaps you can include a default case for switch statements.

zhizhizhizhizhi · 2020-09-01T12:49:20Z

src/main/java/ui/Ui.java

@@ -55,7 +55,8 @@ public void displayErrorMessage(Exception ex){
     * Show all the tasks that are still in the list


Perhaps it should be "Shows all...". This is seen in the javadocs comments for other methods in this file and other files too.

zhizhizhizhizhi · 2020-09-01T12:51:55Z

src/main/java/commands/FindCommand.java

+public class FindCommand extends Command {
+    private final String keyword;
+
+    public FindCommand(String keyword){


There should be whitespace before '{'. This is seen in other block statements too.

* FXML: Complete ui Gradle function

Update code based on coding standard

* 'master' of https://github.com/SherryWu178/ip: Update code based on coding standard

* master: Update code based on coding standard # Conflicts: # src/main/java/duke/Duke.java # src/main/java/duke/parser/Parser.java # src/main/java/duke/ui/Ui.java

Add assertions

* 'master' of https://github.com/SherryWu178/ip: Add assertions

* branch-DoWithinPeriodTasks: Add PeriodTask class

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

Add Gradle support

3b19ba1

Complete level1

2353fe0

Add list function (lvl2)

7ec4d59

Add markasdone function and task class(lvl3)

9b96c59

Add todo, deadline and event (lvl4)

7ef1753

Implement testing

962710e

Add exception handling (lvl5)

85d9fac

Add delete function (lvl6)

cd1250c

Merge commit '3b19ba1790d6035a9e246f5d4ae627bf68778606'

d57c06f

Use enum for different command

133b571

Add local storage for the tasks

9d63024

Add data.txt

d7114d3

Add content for data.txt

f932a96

Change time field in Deadline and Event class to a LocalDate object

b954a33

Merge branch 'branch-Level-7'

19c2ddb

* branch-Level-7: Add local storage for the tasks

Merge branch 'branch-Level-8'

b3cd852

* branch-Level-8: Change time field in Deadline and Event class to a LocalDate object Add content for data.txt Add data.txt # Conflicts: # src/main/java/Deadline.java # src/main/java/Duke.java # src/main/java/Event.java # src/main/java/Todo.java

Fix data.txt not found situation

6b74578

Add Command and its subclasses, Ui, TaskList, and Parser

542ebb8

Add package

118650e

Complete package

9d3cf6d

Complete test for Parser

9c02bf1

Create JAR file

5398d23

SherryWu178 force-pushed the master branch from adb53da to 5398d23 Compare August 25, 2020 14:57

SherryWu178 added 7 commits August 25, 2020 23:05

Include WrongDateFormatException

ab4a0db

Add find function

a0981c0

Add Javadoc for some methods

fc86a18

Fix syntax error

baec05f

Merge branch 'branch-Level-9'

8e4817b

* branch-Level-9: Add find function

Merge branch 'branch-A-JavaDo'

53d5556

* branch-A-JavaDo: Add Javadoc for some methods # Conflicts: # src/main/java/tasks/Task.java

Merge branch 'A-CodingStandard'

ec28ece

* A-CodingStandard: Fix syntax error # Conflicts: # src/main/java/parser/Parser.java # src/main/java/ui/Ui.java

Modify files for gradle

2344d3b

erisjacey reviewed Aug 31, 2020

View reviewed changes

SherryWu178 added 3 commits August 31, 2020 20:36

Add javafx

ba2045f

Set up controllers and views to use FXML

5f89175

Gradle function

c39320d

zhizhizhizhizhi reviewed Sep 1, 2020

View reviewed changes

SherryWu178 and others added 20 commits September 1, 2020 21:46

Complete ui

457b0d8

Merge branch 'FXML'

7234650

* FXML: Complete ui Gradle function

Fix checkstyle errors

2b1bc56

Add assertions

4a13c80

Update code based on coding standard

9bf776e

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

c2b0a5d

Update code based on coding standard

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

5b47c10

* 'master' of https://github.com/SherryWu178/ip: Update code based on coding standard

Merge branch 'master' into branch-A-Assertions

262e91c

* master: Update code based on coding standard # Conflicts: # src/main/java/duke/Duke.java # src/main/java/duke/parser/Parser.java # src/main/java/duke/ui/Ui.java

Merge pull request #3 from SherryWu178/branch-A-Assertions

da78b78

Add assertions

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

2e139f9

* 'master' of https://github.com/SherryWu178/ip: Add assertions

Add PeriodTask class

52c6618

Merge branch 'branch-DoWithinPeriodTasks'

4afd41c

* branch-DoWithinPeriodTasks: Add PeriodTask class

Refactor the parser code

3384e95

Refactor into different parsers and add help text

576d486

Improve UI

11a6682

Add user guide

57d93dc

Fix checkstyle errors

09f2973

Add picture into README

ee024a4

Update user guide

0e29f9b

Fix bug with date search with period-task; add DateSequenceException

09fa715

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Wu Yujin] iP #155

[Wu Yujin] iP #155

SherryWu178 commented Aug 22, 2020

erisjacey left a comment

erisjacey Aug 31, 2020

erisjacey Aug 31, 2020

erisjacey Aug 31, 2020

erisjacey Aug 31, 2020

erisjacey Aug 31, 2020

erisjacey Aug 31, 2020

erisjacey Aug 31, 2020

erisjacey Aug 31, 2020

erisjacey Aug 31, 2020

erisjacey Aug 31, 2020

zhizhizhizhizhi left a comment

zhizhizhizhizhi Sep 1, 2020

zhizhizhizhizhi Sep 1, 2020

zhizhizhizhizhi Sep 1, 2020

zhizhizhizhizhi Sep 1, 2020

		return (type.equals(command.type))&&(description.equals(command.description))
		&&(date == date\|\|date.equals(command.date));

		protected boolean hasTime;
		protected static int numberOfTasks = 0;

		@@ -0,0 +1,34 @@
		package test.java.parser;

		import main.java.commands.*;

		@@ -55,7 +55,8 @@ public void displayErrorMessage(Exception ex){
		* Show all the tasks that are still in the list

[Wu Yujin] iP #155

Are you sure you want to change the base?

[Wu Yujin] iP #155

Conversation

SherryWu178 commented Aug 22, 2020

erisjacey 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhizhizhizhizhi 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