Skip to content

[Chanell Ng] iP#244

Open
chanellNg wants to merge 63 commits into
nus-cs2103-AY2021S2:masterfrom
chanellNg:master
Open

[Chanell Ng] iP#244
chanellNg wants to merge 63 commits into
nus-cs2103-AY2021S2:masterfrom
chanellNg:master

Conversation

@chanellNg
Copy link
Copy Markdown

No description provided.

}else {
createBot("data/duke.txt", bot);
}
}catch (IOException e){
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Spacing

@jared98lyj
Copy link
Copy Markdown

Concise code, only importing necessary libraries. Good abstraction of printing functions like echo. Overall structure of code is good, with a few formatting errors here and there.

Copy link
Copy Markdown

@weixue123 weixue123 left a comment

Choose a reason for hiding this comment

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

Hello, I find that there were parts of your code that made good sense and were very readable. Great job!

Ui ui = new Ui(bot);

while (!(input.equals("bye"))) {
if (input.equals("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.

Perhaps the code can be more readable if the IF/ELSE statements are written in a way that is less nested?

Comment thread src/main/java/ip/src/main/java/Ui.java Outdated




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 feel that the use of line breaks can be more consistent! For example, whether or not to include an empty line at the EOF, or how many of such empty lines to put, etc.

package ip.src.main.java;
import java.util.Scanner;

public class Level_1 {
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 consider deleting dead code? So that your codebase can be simpler and easier to understand.

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.

4 participants