Skip to content

[Guanlin] iP#187

Open
Fureimi wants to merge 44 commits into
nus-cs2113-AY2324S2:masterfrom
Fureimi:master
Open

[Guanlin] iP#187
Fureimi wants to merge 44 commits into
nus-cs2113-AY2324S2:masterfrom
Fureimi:master

Conversation

@Fureimi
Copy link
Copy Markdown

@Fureimi Fureimi commented Feb 8, 2024

No description provided.

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

public static void main(String[] args) {
String WelcomeMessage = "____________________ \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.

Perhaps can use a constant or a method for the divider line

Comment thread src/main/java/GermaBot.java Outdated
import java.util.Scanner;

public class GermaBot {
public static int getIdx(String input) {
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 can rename to getIndex to make it more readable when as your code gets much bigger

Comment thread src/main/java/GermaBot.java Outdated
String echo;
Scanner in = new Scanner(System.in);
echo = in.nextLine();
if (echo.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.

A bit nitpicky but you can include your goodbye message code here. Would make the code more readable.

Comment thread src/main/java/GermaBot.java Outdated
printCounter++;
}
} else if (echo.contains("unmark")) {
int idx = getIdx(echo);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similar to the comment for getIndex. Perhaps can name "idx" to some other variable name which is more easy to follow when debugging.

Copy link
Copy Markdown

@NeoMinWei NeoMinWei 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 on your coding standards, however there are some code quality improvements to be made

Comment thread src/main/java/MainRuntime/GermaBot.java Outdated

public class GermaBot {
static int counter = 0;
static final 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.

good job on declaring constant

Comment thread src/main/java/MainRuntime/GermaBot.java Outdated
}

public static void createTodo(String input) throws EmptyTaskException {
String toDoTask = input.substring(input.indexOf("todo ") + 5);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what is 5? do avoid magic numbers

Comment thread src/main/java/MainRuntime/GermaBot.java Outdated
if (idxOfEndDate == -1) {
throw new MissingDeadlineException();
}
String date = description.substring( idxOfEndDate + 4);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what does 4 here stand for? do check through your code for more magic numbers

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
String date = description.substring( idxOfEndDate + 4);
String date = description.substring(idxOfEndDate + 4);

Comment thread src/main/java/MainRuntime/GermaBot.java Outdated
Comment on lines +97 to +99
}

}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why is there a newline between these 2 brackets?

Comment thread src/main/java/MainRuntime/GermaBot.java Outdated
Comment on lines +132 to +142
} else {
int printCounter = 1;
System.out.println("Gotcha! Here are your tasks:");
for (int i = 0; i < counter; i++) {
if (toDoList[i] == null) {
break;
}
System.out.println(printCounter + ". " + toDoList[i].toString());
printCounter++;
}
}
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 part is deeply nested, try to avoid deep nesting

}
}

public static void main(String[] args) {
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 method is very long, try to shorten it

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