Skip to content

[JonathanOngJoe] iP#175

Open
Jonathan2745 wants to merge 41 commits intonus-cs2113-AY2425S2:masterfrom
Jonathan2745:master
Open

[JonathanOngJoe] iP#175
Jonathan2745 wants to merge 41 commits intonus-cs2113-AY2425S2:masterfrom
Jonathan2745:master

Conversation

@Jonathan2745
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@Li-JunXian Li-JunXian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work overall, your code has a very clear logic chain. It feels very smooth reading it.

Comment on lines +3 to +23
class Task {
private final String description;
private boolean isDone;

public Task(String description) {
this.description = description;
this.isDone = false;
}

public void markAsDone() {
isDone = true;
}

public void markAsNotDone() {
isDone = false;
}

public String getStatusIcon() {
return (isDone ? "[X] " : "[ ] ") + description;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well coded! Simple and straight forward, maybe perhaps we can try creating a separate class file for "Task" so everything is not placed in one single java file.

private int taskCount = 0;

public void start() {
System.out.println("____________________________________________________________");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the horizontal lines can be assigned to a variable for easier reference and usage in the future as compared to inputting it every time.

}
System.out.println("____________________________________________________________");
} else if (input.startsWith("mark ")) {
int index = Integer.parseInt(input.substring(5)) - 1;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be much more clearer to include comments on the meaning of "input.substring(5)) - 1" statement so that when looking back on the code in the future, everyone will understand the significance of the "- 1" for instance.

tasks[index].markAsNotDone();
System.out.println("____________________________________________________________");
System.out.println(" OK, I've marked this task as not done yet:");
System.out.println(" " + tasks[index].getStatusIcon());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is best not to start a System.out.printlin with a space or a tab, perhaps you can place the space and tab at the previous print statement's tail.

System.out.println("____________________________________________________________");
} else if (input.startsWith("mark ")) {
int index = Integer.parseInt(input.substring(5)) - 1;
if (index >= 0 && index < taskCount) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this line appears quite often, perhaps we could position this line with the higher-up if statement so that it can be checked earlier for the program to determine if it is a valid input or not and then execute the correct corresponding actions without doing any redundant work.

Copy link
Copy Markdown

@Deanson-Choo Deanson-Choo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Job Overall! A few touchups will do!

@@ -0,0 +1,97 @@
import java.util.Scanner;

class Task {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can consider defining your classes such as Task in a separate file.

Comment on lines +38 to +82
while (true) {
input = scanner.nextLine();

if (input.equalsIgnoreCase("bye")) {
break;
} else if (input.equalsIgnoreCase("list")) {
System.out.println("____________________________________________________________");
System.out.println(" Here are the tasks in your list:");
if (taskCount == 0) {
System.out.println(" No tasks added yet.");
} else {
for (int i = 0; i < taskCount; i++) {
System.out.println((i + 1) + ". " + tasks[i].getStatusIcon());
}
}
System.out.println("____________________________________________________________");
} else if (input.startsWith("mark ")) {
int index = Integer.parseInt(input.substring(5)) - 1;
if (index >= 0 && index < taskCount) {
tasks[index].markAsDone();
System.out.println("____________________________________________________________");
System.out.println(" Nice! I've marked this task as done:");
System.out.println(" " + tasks[index].getStatusIcon());
System.out.println("____________________________________________________________");
}
} else if (input.startsWith("unmark ")) {
int index = Integer.parseInt(input.substring(7)) - 1;
if (index >= 0 && index < taskCount) {
tasks[index].markAsNotDone();
System.out.println("____________________________________________________________");
System.out.println(" OK, I've marked this task as not done yet:");
System.out.println(" " + tasks[index].getStatusIcon());
System.out.println("____________________________________________________________");
}
} else {
if (taskCount < 100) {
tasks[taskCount++] = new Task(input);
System.out.println("____________________________________________________________");
System.out.println(" added: " + input);
System.out.println("____________________________________________________________");
} else {
System.out.println(" Task list is full!");
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you can consider adding another class to manage your tasks to neaten the code

Comment on lines +12 to +23
public void markAsDone() {
isDone = true;
}

public void markAsNotDone() {
isDone = false;
}

public String getStatusIcon() {
return (isDone ? "[X] " : "[ ] ") + description;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you can consider combining markAsNotDone and markAsDone under a single function such as setStatus. Also, should there be a getter function for description and isDone?

private int taskCount = 0;

public void start() {
System.out.println("____________________________________________________________");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps considering assigning the horizontal line to a variable for easier usage

Comment on lines +38 to +82
while (true) {
input = scanner.nextLine();

if (input.equalsIgnoreCase("bye")) {
break;
} else if (input.equalsIgnoreCase("list")) {
System.out.println("____________________________________________________________");
System.out.println(" Here are the tasks in your list:");
if (taskCount == 0) {
System.out.println(" No tasks added yet.");
} else {
for (int i = 0; i < taskCount; i++) {
System.out.println((i + 1) + ". " + tasks[i].getStatusIcon());
}
}
System.out.println("____________________________________________________________");
} else if (input.startsWith("mark ")) {
int index = Integer.parseInt(input.substring(5)) - 1;
if (index >= 0 && index < taskCount) {
tasks[index].markAsDone();
System.out.println("____________________________________________________________");
System.out.println(" Nice! I've marked this task as done:");
System.out.println(" " + tasks[index].getStatusIcon());
System.out.println("____________________________________________________________");
}
} else if (input.startsWith("unmark ")) {
int index = Integer.parseInt(input.substring(7)) - 1;
if (index >= 0 && index < taskCount) {
tasks[index].markAsNotDone();
System.out.println("____________________________________________________________");
System.out.println(" OK, I've marked this task as not done yet:");
System.out.println(" " + tasks[index].getStatusIcon());
System.out.println("____________________________________________________________");
}
} else {
if (taskCount < 100) {
tasks[taskCount++] = new Task(input);
System.out.println("____________________________________________________________");
System.out.println(" added: " + input);
System.out.println("____________________________________________________________");
} else {
System.out.println(" Task list is full!");
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you can consider adding comments to each of your conditions too!

Copy link
Copy Markdown

@aditichadha1310 aditichadha1310 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall, good work!

try {
switch (command) {
case "bye":
System.out.println("____________________________________________________________");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this line of code is repeating at multiple instances, you might want to save it in a variable or make it a function which you can reference.

private final Scanner scanner = new Scanner(System.in);
private final ArrayList<Task> tasks = new ArrayList<>();

public void start() throws InputExceptions {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid lengthy methods. try to keep each method to about 30 lines of code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants