Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[paulktham] iP #104

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Conversation

paulktham
Copy link

No description provided.

Copy link

@limkongkiat limkongkiat left a comment

Choose a reason for hiding this comment

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

Do refer to the coding style guide and notes on readability to improve the overall readability of code.

System.out.println(" see you brother");
System.out.println(Constants.LINE);
}
public static void getUserInput(String userInput){

Choose a reason for hiding this comment

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

Methods should be kept within 30 LoCs. You can extract parts of your code as different methods.

import java.util.Scanner;

public class Tyrone {
public class Constants{

Choose a reason for hiding this comment

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

Constants class should be defined in a new file for better readability

}
}
}
public static void main(String[] args) {

Choose a reason for hiding this comment

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

Line spacing can be used to separate code into more "logical" chunks (e.g. add spacing between different methods)

switch (userInput){
case "list":
System.out.println(Constants.LINE);
for (int i=1; i<listCount+1;i++){

Choose a reason for hiding this comment

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

Inconsistent spacing

Copy link

@wenxin-c wenxin-c 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 job. Just a few suggestions for you to consider.

import tyrone.task.ToDo;

public class Tyrone {
public static void getUserInput(String userInput) throws TyroneException {

Choose a reason for hiding this comment

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

This method is very long, you may consider extracting each case into separate methods. It will be better to keep methods short.

}
Constants.goodbye();
}
}

Choose a reason for hiding this comment

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

Please leave a new line at the end of each page.

Task task = toDoList.get(i);
String statusIcon = task.getStatusIcon();
if (task instanceof Deadline) {
System.out.println(" " + (i + 1) + ". [D][" + statusIcon + "] " + task.getDescription() + " (by: " + ((Deadline) task).getDoBy() + ")");

Choose a reason for hiding this comment

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

You may consider wrapping the line if it gets too long.

Comment on lines +29 to +33
toDoList.get(index).setDone(true);
System.out.println(LINE);
System.out.println(" TIGHT! I've marked this task as done:");
System.out.println(" [X] " + toDoList.get(index).getDescription());
System.out.println(LINE);

Choose a reason for hiding this comment

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

You can consider extracting this part into a method and pass in parameters to differentiate with unmarkAsUndone as they are pretty similar.

Comment on lines 43 to 47
System.out.println(Constants.LINE);
System.out.println(" Got it. I've added this task:");
System.out.println(" [D][ ] " + deadlineTask.getDescription() + " (by: " + deadlineTask.getDoBy() + ")");
System.out.println(" Now you have " + Task.listCount + " tasks in the list.");
System.out.println(Constants.LINE);

Choose a reason for hiding this comment

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

You may consider extracting the printing output part into a method as the similar pattern has been used in a few other methods.

System.out.println(" Noted. I've removed this task:");
String taskType = removedTask instanceof Deadline ? "D" : removedTask instanceof Event ? "E" : "T";
String statusIcon = removedTask.getStatusIcon();
System.out.println(" [" + taskType + "][" + statusIcon + "] " + removedTask.getDescription());

Choose a reason for hiding this comment

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

You may consider extracting the indentation into a named constant as it's used quite often in the 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.

3 participants