Skip to content

[Liu Xuyan] iP#185

Open
Flaaaash wants to merge 26 commits into
nus-cs2113-AY2425S2:masterfrom
Flaaaash:master
Open

[Liu Xuyan] iP#185
Flaaaash wants to merge 26 commits into
nus-cs2113-AY2425S2:masterfrom
Flaaaash:master

Conversation

@Flaaaash
Copy link
Copy Markdown

@Flaaaash Flaaaash commented Feb 6, 2025

Add Task class to manage tasks.
Add addTask() method to add tasks.
Add List() method to allow users to view the stored tasks.
Add markTask() and unmarkTask() to manage the status of a task.

Comment thread src/main/java/Flaaaash.java Outdated

private static void markTask(int index) {
System.out.println("____________________________________________________________");
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.

The condition should be index <= 0 || index > taskCount to properly handle cases where index = 0.

Comment thread src/main/java/Event.java Outdated

@Override
public String toString() {
return "[E]" + super.toString() + " (from: " + from + " to: " + to + ")";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lack of spacing after "[E]" reduces readability.I think it's better if can add return "[E] " + super.toString() + " (from: " + from + " to: " + to + ")"; like this

Comment thread src/main/java/Flaaaash.java Outdated
default:
repeat(command);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The class is well-structured with clear separation of concerns.

Comment thread src/main/java/Task.java Outdated
@@ -0,0 +1,26 @@
public class Task {
protected String taskName;
protected boolean isDone;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good naming of boolean variables

Comment thread src/main/java/Task.java Outdated

public void unmarkAsDone() {
this.isDone = false;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can use a single setter public void setDone(boolean isDone) here

Comment thread src/main/java/Deadline.java Outdated
@@ -0,0 +1,13 @@
public class Deadline extends Task {
protected String deadLine;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Depends on how you view the word Deadline, but if this is one word, it should be all lowercase like deadline

Comment thread src/main/java/Flaaaash.java Outdated
public class Flaaaash {
private static final int maxCount = 100;
private static final Task[] tasks = new Task[maxCount];
private static int taskCount = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good naming standard for the tasks array

Comment thread src/main/java/Todo.java Outdated
Comment on lines +1 to +10
public class Todo extends Task {
public Todo(String taskName) {
super(taskName);
}

@Override
public String toString() {
return "[T]" + super.toString();
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it the same as Task class?

Comment thread src/main/java/Deadline.java Outdated

@Override
public String toString() {
return "[D]" + super.toString() + " (by: " + deadLine + ")";
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 noticed the same issue in several other places too as himethcodes mentioned(spacing)

Comment thread src/main/java/Deadline.java Outdated
@@ -0,0 +1,13 @@
public class Deadline extends Task {
protected String deadLine;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can just name it deadline cause it is single word

Comment thread src/main/java/Flaaaash.java Outdated
break;

case "deadline":
String[] deadlineInputDetails = inputParts[1].split(" /by ", 2);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can handle with the error with splitting string

Comment thread src/main/java/Flaaaash.java Outdated
Comment on lines +4 to +5
private static final int maxCount = 100;
private static final Task[] tasks = new Task[maxCount];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoiding the use of magic number very well

Copy link
Copy Markdown

@tongkiankiat tongkiankiat left a comment

Choose a reason for hiding this comment

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

Code is well structured and neat, good job adding in switch case statements to introduce different methods for different inputs!

Comment thread src/main/java/Flaaaash.java Outdated
greet();
Scanner scanner = new Scanner(System.in);

while (true) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice use of switch-case statements to avoid deep nesting!

Comment thread src/main/java/Flaaaash.java Outdated
tasks[index - 1].markAsDone();
System.out.println(" " + tasks[index - 1]);
}
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.

This magic string could be stored in a variable, since it is used multiple times

Comment thread src/main/java/Flaaaash.java Outdated
}

private static void addTask(Task task) {
tasks[taskCount] = 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.

Good naming convention for singular nouns and plural nouns.

Comment thread src/main/java/Flaaaash.java Outdated
System.out.println("____________________________________________________________");
System.out.println(" Got it. I've added this task:");
System.out.println(" " + tasks[taskCount - 1]);
System.out.println(" Now you have " + taskCount + " tasks in the list.");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could add in the different spelling for 'task' when there is only 1 task and when there are multiple tasks

Comment thread src/main/java/Flaaaash.java Outdated
System.out.println(" " + tasks.get(index - 1));
tasks.remove(tasks.get(index - 1));
System.out.println(" Now you have " + tasks.size() + " tasks in the list.");
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.

Avoid repeating this code line. Store it in a variable or make it a function and use it multiple times.

Comment thread src/main/java/Flaaaash.java Outdated
}
}

private static void loadFromFile() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Keep the method length to about 30 lines of code

Comment thread src/main/java/Flaaaash.java Outdated
Comment on lines +31 to +43
try {
Scanner fileScanner = new Scanner(file);
while (fileScanner.hasNextLine()) {
String line = fileScanner.nextLine();
String[] parts = line.split(" / ");

try {
String type = parts[0];
boolean isDone = parts[1].equals("1");
String description = parts[2];

switch (type) {
case "T":
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 the arrow head style 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.

6 participants