Skip to content

[Timothy Chang] iP#82

Open
timchang27 wants to merge 12 commits into
nus-cs2113-AY2122S2:masterfrom
timchang27:master
Open

[Timothy Chang] iP#82
timchang27 wants to merge 12 commits into
nus-cs2113-AY2122S2:masterfrom
timchang27:master

Conversation

@timchang27

Copy link
Copy Markdown

No description provided.

Comment thread src/main/java/Deadline.java Outdated
@@ -0,0 +1,17 @@
public class Deadline extends Task {
protected String ddate;

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 could use a more descriptive name for this? For example dueDate instead of just ddate.

Comment thread src/main/java/Duke.java Outdated
@@ -1,10 +1,104 @@
import java.util.*;

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 could import just the ArrayList instead of everything from the Java util library?

Comment thread src/main/java/Event.java Outdated
@@ -0,0 +1,16 @@
public class Event extends Task{
protected String edate;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same as previous comment, maybe could use more specific variable name?

Comment thread src/main/java/Task.java Outdated
public class Task {

protected String description;
protected boolean isTaskDone;

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 like the naming of this boolean variable, it is clear what it is representing.

Comment thread src/main/java/Task.java
this.isTaskDone = isTaskDone;
}

public String getDescription(){

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 like how you follow the Java coding standards and stuck with K&R brackets, as well as writing your methods in camelCase

@Nineves Nineves left a comment

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 job! Some details may be improved.

Comment thread src/main/java/Duke.java Outdated
+ "| |_| | |_| | < __/\n"
+ "|____/ \\__,_|_|\\_\\___|\n";

ArrayList<Task> tasks = new ArrayList<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.

Like the use of dynamic ArrayList

Comment thread src/main/java/Duke.java Outdated
System.out.println(tasks.get(index - 1).getDescription());
}
}
else if(userInput.startsWith("todo")){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Clean code, could think about refactoring the statements into functions to demonstrate SLAP.

Comment thread src/main/java/Deadline.java Outdated
@@ -0,0 +1,17 @@
public class Deadline extends Task {
protected String ddate;

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 also think dueDate is a better variable name

@xseh xseh left a comment

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 attempt! Do take note of the Java coding standards and improve code readability with more SLAP.

Comment thread src/main/java/duke/Duke.java Outdated
Comment on lines +8 to +9
import java.util.*;
import java.lang.*;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to the Java standard code,

Imported classes should always be listed explicitly.

Comment thread src/main/java/duke/Duke.java Outdated
Comment on lines +12 to +129
public static void main(String[] args) throws IndexOutOfBoundsException {

Scanner sc = new Scanner(System.in);
DukeException ex = new DukeException();

String logo = " ____ _ \n"
+ "| _ \\ _ _| | _____ \n"
+ "| | | | | | | |/ / _ \\\n"
+ "| |_| | |_| | < __/\n"
+ "|____/ \\__,_|_|\\_\\___|\n";

ArrayList<Task> tasks = new ArrayList<Task>();

System.out.println("Hello from\n" + logo);
System.out.println("Hello! I'm Duke.Duke\n" + "What can I do for you?");
displayOptions();
String userInput;
do {
userInput = sc.nextLine();
if(userInput.equals("list")){
System.out.println("-------------------------------------------------------------------------\n" + "Here are the tasks in your list:");

int i=1;
for(Task task: tasks){
System.out.println(i + ": " + task.getDescription());
i++;
}
i=0;
System.out.println("-------------------------------------------------------------------------");
}
else if(userInput.equals("bye")){
break;
}
else if(userInput.startsWith("mark")){
String[] input = new String[10];
input = userInput.split(" ");
int index = Integer.parseInt(input[1]); //index of task to be marked
if(tasks.get(index-1).getTaskStatus() == true){
System.out.println("The task is already marked!");
System.out.println("-------------------------------------------------------------------------");
}
else{
tasks.get(index-1).setMark();
System.out.println("-------------------------------------------------------------------------\n" + "Nice! I've marked this task as done: ");
System.out.println(tasks.get(index-1).getDescription());
}
}
else if(userInput.startsWith("unmark")){
String[] input = new String[10];
input = userInput.split(" ");
int index = Integer.parseInt(input[1]); //index of task to be unmarked
if(tasks.get(index-1).getTaskStatus()==false){
System.out.println("The task is not completed yet!");
System.out.println("-------------------------------------------------------------------------");
}
else {
tasks.get(index - 1).setUnmark();
System.out.println("-------------------------------------------------------------------------\n" + "OK, I've marked this task as not done yet:");
System.out.println(tasks.get(index - 1).getDescription());
}
}
else if(userInput.startsWith("todo")){
try{
String newtodo = userInput.split("todo")[1].trim();
Todo newTodo = new Todo(newtodo);
tasks.add(newTodo);
System.out.println("-------------------------------------------------------------------------");
System.out.println("Got it! I've added this task: " + newTodo.getDescription());
System.out.println("Now you have " + tasks.size() + " tasks in the list.");
} catch(IndexOutOfBoundsException e){
ex.missingDescription("todo");
} finally{
System.out.println("What else should I do for you?");
}
}
else if(userInput.startsWith("deadline")){
try{
String[] input = userInput.split("/by");
String ddate = input[1].trim();
String deadline = input[0].split("deadline")[1].trim();
Deadline newDeadline = new Deadline(deadline, ddate);
tasks.add(newDeadline);
System.out.println("-------------------------------------------------------------------------");
System.out.println("Got it! I've added this task: ");
System.out.println(newDeadline.getDescription());
System.out.println("Now you have " + tasks.size() + " tasks in the list.");
} catch(IndexOutOfBoundsException e){
ex.missingDescription("deadline");
} finally{
System.out.println("What else should I do for you?");
}
}
else if(userInput.startsWith("event")){
try{
String[] input = userInput.split("/at");
String eventDate = input[1].trim();
String event = input[0].split("event")[1].trim();
Event newEvent = new Event(event, eventDate);
tasks.add(newEvent);
System.out.println("-------------------------------------------------------------------------");
System.out.println("Got it! I've added this task: ");
System.out.println(newEvent.getDescription());
System.out.println("Now you have " + tasks.size() + " tasks in the list.");
} catch(IndexOutOfBoundsException e){
ex.missingDescription("event");
} finally{
System.out.println("What else should I do for you?");
}
}
else{
Task newTask = new Task(userInput);
tasks.add(newTask);
System.out.println("added: "+ userInput);
}
}while(!userInput.equals("bye"));

System.out.println("Bye. Hope to see you again soon!");
}

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 long methods (> 30 LoCs). Consider refactoring and SLAP to improve code readability.

Comment thread src/main/java/duke/Duke.java Outdated
do {
userInput = sc.nextLine();
if(userInput.equals("list")){
System.out.println("-------------------------------------------------------------------------\n" + "Here are the tasks in your 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.

Avoid magic literals. Consider putting the constants into variables. For example, "-------------------------------------------------------------------------\n" can be declared as MESSAGE_BORDER.

Comment thread src/main/java/duke/Duke.java Outdated
System.out.println(i + ": " + task.getDescription());
i++;
}
i=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.

Avoid magic numbers. If the value is an iterator, consider putting into a for loop instead.

Comment thread src/main/java/duke/Duke.java Outdated
Comment on lines +41 to +42
}
else if(userInput.equals("bye")){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to the Java coding standard, the if-else statements should resemble:

if (condition) {
    statements;
} else if (condition) {
    statements;
} else {
    statements;
}

Comment thread src/main/java/duke/Duke.java Outdated
String[] input = new String[10];
input = userInput.split(" ");
int index = Integer.parseInt(input[1]); //index of task to be marked
if(tasks.get(index-1).getTaskStatus() == 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.

Avoid complicated expressions. Consider refactoring the expressions into different methods.

Comment thread src/main/java/duke/Duke.java Outdated
else if(userInput.startsWith("mark")){
String[] input = new String[10];
input = userInput.split(" ");
int index = Integer.parseInt(input[1]); //index of task to be marked

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 magic numbers. Instead of a comment, consider declaring a variable for the integers (e.g. INDEX_MARK_TASK)

Comment thread src/main/java/duke/Duke.java Outdated
}
else {
tasks.get(index - 1).setUnmark();
System.out.println("-------------------------------------------------------------------------\n" + "OK, I've marked this task as not done yet:");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to the Java coding standard,

Line length should be no longer than 120 chars.
If the line exceeds the limit, use line wrapping at appropriate places of the line.

Comment thread src/main/java/duke/Duke.java Outdated
Comment on lines +89 to +97
String[] input = userInput.split("/by");
String ddate = input[1].trim();
String deadline = input[0].split("deadline")[1].trim();
Deadline newDeadline = new Deadline(deadline, ddate);
tasks.add(newDeadline);
System.out.println("-------------------------------------------------------------------------");
System.out.println("Got it! I've added this task: ");
System.out.println(newDeadline.getDescription());
System.out.println("Now you have " + tasks.size() + " 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.

Consider further SLAP to improve code readability.

Comment thread src/main/java/duke/DukeException.java Outdated
package duke;

public class DukeException extends Exception {
public void missingDescription(String 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.

According to the Java coding standard, there should be a whitespace between the brackets ){.

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