[Chan Jing Rong] Duke Increments by rongrongrr · Pull Request #346 · nus-cs2103-AY1920S1/duke

rongrongrr · 2019-09-02T09:27:09Z

No description provided.

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

Change file mode on `gradle` to be executable (nus-cs2103-AY1920S1#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

… input

…ent classes

chrischenhui · 2019-09-02T18:14:55Z

I like your commit messages. They all begin with a base verb and are very informative and consistent.

chrischenhui

Your import lines should not have new lines between them

chrischenhui

I like the overall structure of your code. Keep up the good work :)

chrischenhui · 2019-09-02T18:22:26Z

+    public void execute(TaskList tasks) {
+        String command = scanner.nextLine();
+        showLine();
+        if (command.equals("bye")) {


Consider using switches if the if else statements get too messy

chrischenhui · 2019-09-02T18:24:12Z

+
+public class Deadline extends Task {
+    private String name;
+    private boolean done;


Should change done to isDone() to follow variable naming.

chrischenhui · 2019-09-02T18:27:14Z

+
+    public Parser() {
+
+    }


You can choose not to type out the constructor method

chrischenhui · 2019-09-02T18:28:06Z

+                task = new Deadline(words[2], words[3]);
+            } else {
+                task = new Event(words[2], words[3]);
+            }


Consider using switch if your if-else statements are long

chrischenhui · 2019-09-02T18:28:36Z

@@ -0,0 +1,37 @@
+public abstract class Task {
+    private String name;
+    private boolean done;


Done can be renamed isDone

chrischenhui · 2019-09-02T18:29:58Z

@@ -0,0 +1,37 @@
+public abstract class Task {
+    private String name;
+    private boolean done;


Name and done should be protected instead, so that classes that extends it can use it

chrischenhui · 2019-09-02T18:30:45Z

@@ -0,0 +1,17 @@
+public class Todo extends Task {
+    private String name;
+    private boolean done;


Should not recreate a new private variable in Todo. Instead use Task's protected isDone and name

chrischenhui · 2019-09-02T18:31:01Z

+
+    @Override
+    public String toString() {
+        return String.format("[T]%s", super.toString());


I like the use of String.format() here

chrischenhui · 2019-09-02T18:32:24Z

+                int index = Integer.valueOf(commands[1]) - 1;
+                System.out.println("       " + tasks.getTasks().get(index).toString());
+                tasks.delete(index);
+                System.out.println(String.format("     Now you have %d tasks in the list.", tasks.getTasks().size()));


Since you are reusing tasks.getTasks() multiple times, consider creating a variable just to store the reference

chrischenhui · 2019-09-02T18:33:20Z

+        System.out.println();
+    }
+
+    public void execute(TaskList tasks) {


I like the naming of the parameter : tasks, as it suggest it is a list, not one item

sreesubbash · 2019-09-04T15:01:52Z

+     * @return Formatted description of task.
+     */
+    @Override
+    public String toString() {


Good work in abstracting common parts of code and using it in subclasses with super()

sreesubbash · 2019-09-04T15:04:04Z

+                }
+
+                Task task;
+                if (commands[0].equals("todo") || commands[0].equals("deadline") || commands[0].equals("event")) {


Since lines are getting long and commands[0] is used multiple times, maybe assign String move = commands[0] and use move for proceeding lines?

j-lum and others added 17 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-AY1920S1#12 from j-lum/gradle+x

6e6ace1

Change file mode on `gradle` to be executable (nus-cs2103-AY1920S1#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-AY1920S1#13 from j-lum/javaexec

7b60e81

Add configuration for console applications

Add scanner, lines and indentations. Echo commands

3672239

Add arraylist to store tasks. Return appropriate output based on user…

f7d21a1

… input

Create Task class. Mark tasks as done

2eeda50

Convert Task class to abstract class. Implement Todo, Deadline and Ev…

e711463

…ent classes

Handle user input errors

cd13da7

Implement delete function

bac973c

Save tasks in hard disk

96fc35c

Add date and time feature

e5e16e2

Resolve merge conflicts

66a6aa4

Make the code more OOP

acbe16b

Add JUnit testing for Todo, Deadline, Parser, Duke (dummy test)

dbf905d

chrischenhui reviewed Sep 2, 2019

View reviewed changes

rongrongrr added 5 commits September 4, 2019 15:35

Implement find feature

534fdfd

Edit to match coding standard

be2b14f

Add JavaDoc comments to code

d7c47a0

Merge conflicts

daa3220

Merge conflicts

7a98b43

sreesubbash reviewed Sep 5, 2019

View reviewed changes

Merge branch 'gradle'

d469352

nus-se-pr-bot added tutor.zht96830 tutorial.CS2103-T16 team.4 labels Sep 6, 2019

sreesubbash mentioned this pull request Sep 8, 2019

Feedback on week 4 project progress for sreesubbash sreesubbash/duke#4

Open

4 tasks

rongrongrr and others added 14 commits September 9, 2019 23:01

Set up Gradle

14d6de2

Implement GUI

0c42333

Add assertions (#4)

1598a4d

Improve code quality (#5)

f4ff23a

Implement Undo command

d4cfa86

Finalise features

a6cb27b

Finalise features and add cross-platform capabilities

778e691

Set up sample data

3eeb423

Add product representative screenshot

61d55fb

Add User Guide

bf468af

Edit User Guide

2ce87e7

Set theme jekyll-theme-cayman

656740f

Edit storage function

ca6585d

Merge branch 'master' of https://github.com/rongrongrr/duke

10548b3

Conversation

rongrongrr commented Sep 2, 2019

Uh oh!

chrischenhui commented Sep 2, 2019

Uh oh!

chrischenhui left a comment

Choose a reason for hiding this comment

Uh oh!

chrischenhui left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants