[Tham Chun Leong] iP by allyfern72 · Pull Request #87 · nus-cs2113-AY2122S2/ip

allyfern72 · 2022-02-03T19:30:49Z

No description provided.

edemirkirkan

Looks good to merge, just a few little nits.

edemirkirkan · 2022-02-04T01:51:15Z

+public class Bao {
+    private static Scanner in = new Scanner(System.in);
+    private static Task[] tasks = new Task[100];
+    private static int numTasks=0;


Since the 'numTasks' has a static modifier, it must belong to a task class, and should've incremented in while task was added or created, eg. on constructor.

edemirkirkan · 2022-02-04T01:52:13Z

+
+    private static void addToDo(String msg){
+        String description;
+        final int TODO_LENGTH = "todo".length();


you should define these kind of constants at the beginning of the class

edemirkirkan · 2022-02-04T01:52:42Z

+
+    private static void addDeadline(String msg){
+        String description, dateTime;
+        final int DEADLINE_LENGTH = "deadline".length();


you should define these kind of constants at the beginning of the class

edemirkirkan · 2022-02-04T01:52:52Z

+
+    private static void addEvent(String msg){
+        String description, dateTime;
+        final int EVENT_LENGTH = "event".length();


you should define these kind of constants at the beginning of the class

edemirkirkan · 2022-02-04T01:58:09Z

+        return "[D]" +
+                "[" + getStatusIcon() + "] " + description +
+                " (by: " + getDateTime() + ")";


Suggested change

return "[D]" +

"[" + getStatusIcon() + "] " + description +

" (by: " + getDateTime() + ")";

return "[D]" + super.toString() + " (by:" + getDateTime() + ")";

edemirkirkan · 2022-02-04T01:58:59Z

+        return "[E]" +
+                "[" + getStatusIcon() + "] " + description +
+                " (at: " + getDateTime() + ")";


Suggested change

return "[E]" +

"[" + getStatusIcon() + "] " + description +

" (at: " + getDateTime() + ")";

return "[E]" + super.toString() + " (at:" + getDateTime() + ")";

edemirkirkan · 2022-02-04T02:00:05Z

+package Components;
+
+public class Event extends Task{
+    String dateTime;


Suggested change

String dateTime;

protected String dateTime;

all of the field members of the class should be private or protected

edemirkirkan · 2022-02-04T02:00:31Z

+package Components;
+
+public class Deadline extends Task{
+    String dateTime;


Suggested change

String dateTime;

protected String dateTime;

all of the field members of the class should be private or protected

edemirkirkan · 2022-02-04T02:01:13Z

+    }
+
+    public String toString(){
+        return "[T]" + "[" + getStatusIcon() + "] " + description;


Suggested change

return "[T]" + "[" + getStatusIcon() + "] " + description;

return "[T]" + super.toString();

Bryan-BC

Looks great!
Almost everything abides to the coding standard.

Bryan-BC · 2022-02-04T02:43:52Z

+    }
+
+    public String getStatusIcon() {
+        return (isDone ? "X" : " "); // mark done task with X


I like the usage of the ternary operator, makes it look cleaner

Bryan-BC · 2022-02-04T02:48:16Z

+    private static Scanner in = new Scanner(System.in);
+    private static Task[] tasks = new Task[100];
+    private static int numTasks=0;
+


Any reason why you did not include a variable separator so you do not have to print the long line of underscore?

Suggested change

private static String separator = "______________________________________________________________________________________";

Bryan-BC · 2022-02-04T02:49:41Z

+
+    private static void addTask(Task task){
+        System.out.println("______________________________________________________________________________________");
+        tasks[numTasks++]=task;


I think it would be better to have a space separating the equals sign like you did in other lines.

Suggested change

tasks[numTasks++]=task;

tasks[numTasks++] = task;

edemirkirkan · 2022-02-04T03:04:04Z

+    }
+
+    public void setIsDone(boolean isDone){
+        this.isDone=isDone;


you should use spacing before and after assignment statement

YitHien

Good code overall, just a few minor formatting changes. LGTM

YitHien · 2022-02-04T02:49:48Z

+    private static void addDeadline(String msg){
+        String description, dateTime;
+        final int DEADLINE_LENGTH = "deadline".length();
+        description = msg.substring(DEADLINE_LENGTH,msg.indexOf("/by")).trim();


You could simplify this expression!

YitHien · 2022-02-04T02:50:37Z

+        description = msg.substring(EVENT_LENGTH,msg.indexOf("/at")).trim();
+        dateTime = msg.substring(msg.indexOf("/at")+"/at".length()).trim();


You could simplify these expressions!

YitHien · 2022-02-04T02:52:54Z

+        return "[E]" +
+                "[" + getStatusIcon() + "] " + description +
+                " (at: " + getDateTime() + ")";


Nice line breaking to prevent the line from getting too long!

YitHien · 2022-02-04T02:56:50Z

+    }
+
+    private static void markTask(String msg){
+        System.out.println("______________________________________________________________________________________");


You could refactor this printing into a separate method/constant since it is used a lot

Mick609

Readable code with minor issues.

Mick609 · 2022-03-01T03:11:27Z

+import static Constants.BaoConstants.LINE_BREAK;
+
+public class Bao {
+    private static Scanner in = new Scanner(System.in);


Maybe you should consider using more descriptive names like "inScanner" for this object?

Mick609 · 2022-03-01T03:12:29Z

+            try {
+                if (userInput.equalsIgnoreCase("list")) {
+                    listTasks();
+                } else if (userInput.toLowerCase().startsWith ("mark")) {


There is an extra space for the startsWith method.

Mick609 · 2022-03-01T03:15:51Z

+                tasklistFile.createNewFile();
+            }
+
+            Scanner s = new Scanner(tasklistFile);


Maybe a name like "scanner" would be better?

Mick609 · 2022-03-01T03:16:18Z

+
+    static void saveTasklist(ArrayList<Task> tasks) throws IOException {
+        try {
+            FileWriter fw = new FileWriter(FILE_PATH);


A name like "fileWriter" would increase the readability.

Mick609 · 2022-03-01T03:27:08Z

+
+    static void listTasks() {
+        for (int i = 0; i < numTasks; i++) {
+            System.out.println(i+1 + ". " + tasks.get(i).toString());


Maybe you could make the space between operators consistent.

Level-8: Use LocalDateTime to store dates and times

Level-9: Add Find function

A-JavaDoc: Add JavaDoc comments

allyfern72 added 7 commits January 20, 2022 00:32

Add Increment - Level 0: Greet and farewell

68d0bb0

Add Increment - Level 1: Greet, Echo, Exit

a9e1a52

Add Increment - Level 2: Add, List

4c3a44e

Add Increment - Level 3: Mark as Done

2bb701f

Add "Components" package

5ddb811

Add Increment Level-4: ToDo, Event, Deadline

6e22a8b

Tidy Main

696c4d3

edemirkirkan reviewed Feb 4, 2022

View reviewed changes

Bryan-BC approved these changes Feb 4, 2022

View reviewed changes

edemirkirkan reviewed Feb 4, 2022

View reviewed changes

YitHien reviewed Feb 4, 2022

View reviewed changes

allyfern72 added 11 commits February 10, 2022 17:46

Add TaskManager

1926457

Change accessibility modifiers

c6a4ca7

Refactor Bao

43c17ed

Extract constants

5f85a83

Add Level-5: Handle Errors

4faff1b

Merge branch 'branch-Level-5'

971cae1

Add Level-6: Delete task function

5fb9c3e

Add Level-7: Save tasks function

02a42e3

Merge branch 'branch-Level-6'

5e8861e

Add Level-7: Save tasks function

8f95911

Fix bugs

2fbd970

Mick609 reviewed Mar 1, 2022

View reviewed changes

allyfern72 added 7 commits March 3, 2022 01:06

A-MoreOOP: Use more OOP

7edcd22

Level-8: Use LocalDateTime to store dates and times

b4c6ce3

Level-9: Add Find function

21ed93f

A-JavaDoc: Add JavaDoc comments

ed0f2ae

Merge pull request #1 from allyfern72/branch-Level-8

4923a21

Level-8: Use LocalDateTime to store dates and times

A-JavaDoc: Add JavaDoc comments

eb52bbd

Merge branch 'master' into branch-A-JavaDoc

51e51ea

allyfern72 added 15 commits March 4, 2022 05:13

A-JavaDoc: Add JavaDoc comments

d4482ae

Merge pull request #2 from allyfern72/branch-Level-9

9b91f3f

Level-9: Add Find function

Merge branch 'master' into branch-A-JavaDoc

fe80176

Reorder exception catching

c2c4573

Merge pull request #3 from allyfern72/branch-A-JavaDoc

bda4810

A-JavaDoc: Add JavaDoc comments

Refactor UI implementation

1c48791

A-UserGuide: Add a User Guide

9e1cbc0

Set theme jekyll-theme-cayman

2daa49c

Update User Guide

00ef647

Merge remote-tracking branch 'origin/master'

03fdd12

Update User Guide

272d362

A-UserGuide: Add User Guide

3df2e18

Update app README.md

64a02c9

Add Help command

d82328a

A-TextUiTesting: Add automated program test

1607a18

	return "[T]" + "[" + getStatusIcon() + "] " + description;
	return "[T]" + super.toString();


	private static String separator = "______________________________________________________________________________________";

		description = msg.substring(EVENT_LENGTH,msg.indexOf("/at")).trim();
		dateTime = msg.substring(msg.indexOf("/at")+"/at".length()).trim();

Uh oh!

Conversation

allyfern72 commented Feb 3, 2022

Uh oh!

edemirkirkan 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!

Bryan-BC 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!

YitHien 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!

Mick609 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants