Skip to content

iP - #188

Open
VishalJeyaram wants to merge 75 commits into
nus-cs2113-AY2122S1:masterfrom
VishalJeyaram:master
Open

iP#188
VishalJeyaram wants to merge 75 commits into
nus-cs2113-AY2122S1:masterfrom
VishalJeyaram:master

Conversation

@VishalJeyaram

Copy link
Copy Markdown

Levels-0-3 and A-CodingStandard have been added

@YEOWEIHNGWHYELAB YEOWEIHNGWHYELAB 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.

LGTM! There are minor code quality issues that can be fixed easily.

Comment thread src/main/java/Duke.java Outdated
Scanner in = new Scanner(System.in);
while ( byeFlag != 1 ) {
while (byeFlag != 1) {
System.out.println("____________________________________________________________\n");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion:
Make a variable for printing underscore.

Comment thread src/main/java/Duke.java Outdated
// checkCommands() is a method that allows us to determine when the user says bye.
private static void checkCommands(String line) {
if ( positionCheck >= 100 ) {
if (positionCheck >= 100) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion:
Reduce the use of magic numbers.

Comment thread src/main/java/Duke.java Outdated
if ( positionCheck == 0 ) {
if (positionCheck == 0) {
System.out.println("There is no data to show master!");
} else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion:
Avoid deep nesting.

Comment thread src/main/java/Duke.java Outdated
private static Task[] commands = new Task[100];

// sendCommands() is a method used to allow the user to send his/her commands to C3PO
private static void sendCommands() {

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 be changed to sayCommand as there is only 1 command

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

// checkCommands() is a method that allows us to determine when the user says bye.
private static void checkCommands(String line) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similarly for this could be checkCommand

Comment thread src/main/java/Duke.java Outdated
sendCommands();
sayBye();
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Overall, good naming conventions used 💯

Comment thread src/main/java/Duke.java Outdated
return;
}
}
System.out.println("Sorry Master! I don't think you have properly keyed in the parameters. Please enter the task, followed by \"/by\",\n" +

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 be split into 2 println statements to improve readability

@YEOWEIHNGWHYELAB YEOWEIHNGWHYELAB 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.

LGTM! Just make some minor changes.

Comment thread src/main/java/Duke.java Outdated
}
} else if (line.equals("done")){
System.out.println("Oh no master, I am not quite sure which task you would like me to mark as done");
} else if (line.contains("done")){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion:
Could transfer these into another class.

Comment thread src/main/java/Duke.java Outdated
@@ -31,11 +31,19 @@ private static void checkCommands(String line) {
} else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: Avoid deep nesting.

Comment thread src/main/java/Duke.java Outdated
String line;
Scanner in = new Scanner(System.in);
while ( byeFlag != 1 ) {
System.out.println("____________________________________________________________\n");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion:
Make a variable for printing underscore.

Comment thread src/main/java/Duke.java Outdated
// checkCommands() is a method that allows us to determine when the user says bye.
private static void checkCommands(String line) {
if ( positionCheck >= 100 ) {
if (positionCheck >= 100) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion:
Reduce the use of magic numbers.

Comment thread src/main/java/Duke.java Outdated
"followed by the due date to specify the deadline Master!");
}

public static void addEvent(String[] input, int length) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Try to use single level abstraction.

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