Skip to content

[Mikolaj Jedrzejewski] iP#97

Open
mikolajed wants to merge 42 commits into
nus-cs2113-AY2425S1:masterfrom
mikolajed:master
Open

[Mikolaj Jedrzejewski] iP#97
mikolajed wants to merge 42 commits into
nus-cs2113-AY2425S1:masterfrom
mikolajed:master

Conversation

@mikolajed

Copy link
Copy Markdown

No description provided.

damithc and others added 9 commits July 11, 2024 16:52
In build.gradle, the dependencies on distZip and/or distTar causes
the shadowJar task to generate a second JAR file for which the
mainClass.set("seedu.duke.Duke") does not take effect.
Hence, this additional JAR file cannot be run.
For this product, there is no need to generate a second JAR file
to begin with.

Let's remove this dependency from the build.gradle to prevent the
shadowJar task from generating the extra JAR file.

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

looks good overall but some spacing issues.

Comment thread src/main/java/Task.java Outdated
return "[ ] " + name;
}
else {
} 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.

Good changes to maintain code quality standard.

Comment thread src/main/java/Task.java Outdated
this.name = name;
this.status = false;
}

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

Comment thread src/main/java/Pythia.java Outdated
public static void greet() {
String helloMsg = "Welcome, seeker. I am " + botName + ".\n" +
"What brings you here?";
IO.printResponse(helloMsg);

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 looks a little off to me.

Comment thread src/main/java/Pythia.java Outdated
case "list" -> listTasks();
case "add" -> addTask(value);
case "mark" -> markTask(Integer.parseInt(value));
default -> IO.printResponse("Hmm. I am not sure what you mean.");

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 that you handled edge case.

Comment thread src/main/java/Pythia.java Outdated
case "list" -> listTasks();
case "add" -> addTask(value);
case "mark" -> markTask(Integer.parseInt(value));
default -> IO.printResponse("Hmm. I am not sure what you mean.");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nice change to make this much more readable!

Comment thread src/main/java/Pythia.java Outdated
}
else {
} else {
IO.printResponse("There is no such 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.

good change to follow coding standard.

Comment thread src/main/java/Pythia.java Outdated
IO.init();
IO.greet();

boolean byeSaid = false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can change name to be more boolean-like i.e isByeSaid?

Comment thread src/main/java/Pythia.java Outdated
public static void greet() {
String helloMsg = "Welcome, seeker. I am " + botName + ".\n" +
"What brings you here?";
"What brings you here?";

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 think the original looks good already.

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

Awesome job! Your code is both efficient and easy to read. Remember to add comments :)

Comment thread src/main/java/pythia/io/Parser.java Outdated
commandType = parseCommandType(rawText);
argumentList = new ArrayList<>();

switch (commandType) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note there is no indentation for case clauses.

Comment thread src/main/java/pythia/io/Parser.java Outdated
Pattern pattern = Pattern.compile("add\\s(.+)");
Matcher matcher = pattern.matcher(rawText);

String what = "";

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 minimise duplicated codes to improve code quality.

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.

6 participants