[Grant Lee] Duke Increments #161

grrrrnt · 2020-01-29T10:13:08Z

No description provided.

Add toolVersion block in to Gradle code sample to prevent errors.

Change file mode on `gradle` to be executable (#9)

Gradle defaults to an empty stdin which results in runtime exceptions when attempting to read from `System.in`. Let's add some sensible defaults for students who may still need to work with the standard input stream.

Add configuration for console applications

The OpenJFX plugin expects applications to be modular and bundled with jlink, resulting in fat jars that are not cross-platform. Let's manually include the required dependencies so that shadow can package them properly.

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

Capeguy · 2020-02-03T08:48:48Z

src/main/java/Deadline.java

+        this.dayTime = Parser.parseTime(dayTime);
+    }
+
+    public String getDayTime() {


"dayTime" may not be as intuitive of a variable name to use

Capeguy · 2020-02-03T11:11:00Z

src/main/java/EventException.java

@@ -0,0 +1,10 @@
+public class EventException extends DukeException {
+    public EventException() {


Non-intuitive Class Name.

Capeguy · 2020-02-03T11:12:54Z

src/main/java/TodoException.java

@@ -0,0 +1,10 @@
+public class TodoException extends DukeException {


Non-Intuitive Class Name

Capeguy · 2020-02-03T11:13:47Z

src/test/java/TaskTest.java

+    }
+
+    @Test
+    public void getStatusIcon_todo_ok() {


Invalid function name

Test functions should start with the word "test" and use camelCase instead of underlined_function_names

Capeguy · 2020-02-03T11:14:32Z

text-ui-test/input.txt

@@ -0,0 +1,3 @@
+todo buy fruits


Could have more extensive tests

Capeguy · 2020-02-03T11:14:49Z

tutorials/gradleTutorial.md

@@ -1,4 +1,4 @@
-# Gradle Tutorial
+de# Gradle Tutorial


I believe this was an unintentionally commit

# Conflicts: # src/main/java/TaskList.java

RubaP

Please go through the detailed comments given especially handling Duke specific exceptions. You can discuss it with your team as well so that it will benefit you and the team project. You may focus on writing more Junit test cases as well.

RubaP · 2020-02-09T13:45:12Z

src/main/java/DeadlineException.java

+
+    @Override
+    public String getMessage(){
+        return "\u2639 OOPS!!! The description of a deadline cannot be empty.";


Why it is always the same message for all DeadlineException? If this exception is dedicated for empty description, then you can have class name that indicates the purpose. E.g. EmptyDescriptionException

RubaP · 2020-02-09T13:47:25Z

src/main/java/Duke.java

-                + "| |_| | |_| |   <  __/\n"
-                + "|____/ \\__,_|_|\\_\\___|\n";
-        System.out.println("Hello from\n" + logo);
+//        String logo = " ____        _        \n"


Please remove commented code blocks

RubaP · 2020-02-09T13:48:31Z

src/main/java/DukeException.java

+
+    @Override
+    public String getMessage(){
+        return "\u2639 OOPS!!! I'm sorry, but I don't know what that means :-(";


Is this Exception dedicated for unavailable commands? Then you can use a class name which indicates that

RubaP · 2020-02-09T13:50:03Z

src/main/java/EventException.java

+
+    @Override
+    public String getMessage(){
+        return "\u2639 OOPS!!! The description of an event cannot be empty.";


Same goes here. You can either have a generic Exception class which can handle different exceptions or you can make the Class name more intuitive

RubaP · 2020-02-09T13:52:08Z

src/main/java/Parser.java

+        String msg = "";
+        if (input.equals("list")) {
+            msg = taskList.showList();
+        } else if (input.split(" ")[0].equals("done")) {


If input.split(" ")[0] is repeated, again and again, then you can define that as a string and use it.

RubaP · 2020-02-09T13:53:29Z

src/main/java/TaskList.java

+        Task toAdd = new Task();
+        if (typeOfTask.equals("todo")) {
+            try {
+                String task = s.split(" ", 2)[1];


Same goes here. If this logic is repeated, you can extract it out before the if statement

RubaP · 2020-02-09T13:54:16Z

src/main/java/TaskList.java

+     */
+    public String delete(int i) {
+        Task t = taskList.remove(i-1);
+        String msg = "Poof! This task is gone:\n"


You can define these messages as constants if they are not going to change

RubaP · 2020-02-09T13:55:46Z

src/main/java/Ui.java

+     * @param s String to print.
+     */
+    public void print(String s) {
+        System.out.println(s);


Why do you need a separate method only to execute System. out.println

RubaP · 2020-02-09T13:56:00Z

src/main/java/Ui.java

+    public Ui() {}
+
+    /**
+     * Prints string.


You can avoid trivial comments

Add a few assertions

A-CodeQuality

…ain period

Provide support for managing tasks that need to be done within a certain period

j-lum and others added 22 commits August 6, 2019 15:25

Add support for Gradle workflow

65f72a8

Add sample checkstyle configuration

0112efe

Add toolVersion block in to Gradle code sample to prevent errors.

Change file mode on gradle to be executable

cfd6da7

Merge pull request nus-cs2103-AY1920S2#12 from j-lum/gradle+x

6e6ace1

Change file mode on `gradle` to be executable (#9)

Add configuration for console applications

a3ca5a4

Gradle defaults to an empty stdin which results in runtime exceptions when attempting to read from `System.in`. Let's add some sensible defaults for students who may still need to work with the standard input stream.

Merge pull request nus-cs2103-AY1920S2#13 from j-lum/javaexec

7b60e81

Add configuration for console applications

JavaFX tutorial: Support cross-platform JARs

c4678f7

The OpenJFX plugin expects applications to be modular and bundled with jlink, resulting in fat jars that are not cross-platform. Let's manually include the required dependencies so that shadow can package them properly.

JavaFX tutorial: Support cross-platform JARs [nus-cs2103-AY1920S2#16]

30efbae

Complete level 1

6b035d6

Complete level 2

a9df450

Complete level 3

94e2c64

Complete level 4

528c331

Complete level 5

4b9bde2

Complete level 6

ba78bcd

Complete automated text UI testing

ac6e6aa

Complete Level-7

886192d

Complete Level-8

91c538f

Merge master and branch-Level-7

149fb0a

Merge master and branch-Level-8

c0ebbe4

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

Complete A-MoreOOP

0944243

Merge remote-tracking branch 'origin/gradle'

7f4d11a

Add JUnit tests

8801225

Capeguy reviewed Feb 3, 2020

View reviewed changes

nus-se-bot mentioned this pull request Feb 4, 2020

Feedback on week 4 project progress for capeguy Capeguy/duke#9

Open

5 tasks

grrrrnt added 5 commits February 6, 2020 21:56

Add JavaDoc comments

f7a954e

Tweak code to comply with camelCase

67a8283

Complete level 9

6d0990d

Merge branch 'branch-A-JavaDoc'

1820d0d

Merge branch 'branch-A-CodingStandard'

16e5e53

grrrrnt added 3 commits February 6, 2020 22:16

Merge branch 'branch-Level-9'

7e75527

# Conflicts: # src/main/java/TaskList.java

Complete level 10, except getResponse() implementation

51210e7

Complete level 10 (including FXML)

a021641

RubaP reviewed Feb 9, 2020

View reviewed changes

grrrrnt and others added 13 commits February 13, 2020 22:53

Add a few assertions

e4ae347

Merge pull request #5 from grrrrnt/branch-Assertions

f81adae

Add a few assertions

Improve code quality

babb3b1

Merge pull request #7 from grrrrnt/branch-CodeQuality

c7df233

A-CodeQuality

Provide support for managing tasks that need to be done within a cert…

6231264

…ain period

Merge pull request #8 from grrrrnt/branch-DoWithinPeriodTasks

d738272

Provide support for managing tasks that need to be done within a certain period

Set theme jekyll-theme-cayman

2e36822

Add user guide, improve GUI and fix bugs

e7553ed

Merge remote-tracking branch 'origin/master'

b41d8c7

Set theme jekyll-theme-slate

5e827db

Add missing images

1e9edcc

Merge remote-tracking branch 'origin/master'

516efb2

Get final release version ready

c9c1f5c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Grant Lee] Duke Increments #161

[Grant Lee] Duke Increments #161

grrrrnt commented Jan 29, 2020

Capeguy Feb 3, 2020

Capeguy Feb 3, 2020

Capeguy Feb 3, 2020

Capeguy Feb 3, 2020

Capeguy Feb 3, 2020

Capeguy Feb 3, 2020

Capeguy Feb 3, 2020

RubaP left a comment •

edited

Loading

RubaP Feb 9, 2020

RubaP Feb 9, 2020

RubaP Feb 9, 2020

RubaP Feb 9, 2020

RubaP Feb 9, 2020

RubaP Feb 9, 2020

RubaP Feb 9, 2020

RubaP Feb 9, 2020

RubaP Feb 9, 2020

		@@ -0,0 +1,10 @@
		public class EventException extends DukeException {
		public EventException() {

		@@ -0,0 +1,10 @@
		public class TodoException extends DukeException {

[Grant Lee] Duke Increments #161

Are you sure you want to change the base?

[Grant Lee] Duke Increments #161

Conversation

grrrrnt commented Jan 29, 2020

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

RubaP left a comment • edited Loading

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

RubaP left a comment •

edited

Loading