Skip to content

Philip Chang // iP PR#124

Open
philip1304 wants to merge 19 commits into
nus-cs2113-AY2425S1:masterfrom
philip1304:master
Open

Philip Chang // iP PR#124
philip1304 wants to merge 19 commits into
nus-cs2113-AY2425S1:masterfrom
philip1304:master

Conversation

@philip1304

@philip1304 philip1304 commented Sep 13, 2024

Copy link
Copy Markdown

plopBot iP // PR

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

Just some formatting errors and some coding styles to follow for this course! Good work!

Comment thread src/main/java/PlopBot.java Outdated
String command = parts[0].toLowerCase();
String details = parts.length > 1 ? parts[1] : "";

switch (command) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For your switch statements, you could follow the following style set by the couse in this link!

Comment thread src/main/java/PlopBot.java Outdated
}
catch (Exception e) {
}
System.out.println("");

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 you couild indicate a meaningful message for exceptions thrown?

Comment thread src/main/java/PlopBot.java Outdated
catch (IllegalArgumentException ex) {
switch (dateString.toLowerCase()) {
case "today":
return now;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do include break statements, and indicate if its an explicit fallthrough

Comment thread src/main/java/PlopBot.java Outdated
String[] parts = dateTimeString.split(" ", 2);

if (parts.length == 2) {
LocalDate date = parseDateString(parts[0]);

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 it would be good to refactor parts[0] to indicate what it means

Comment thread src/main/java/Task.java Outdated
this.taskName = taskName;
this.type = TaskType.TODO;

//this.isDone = 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.

Do remove unnecessary lines!

Comment thread src/main/java/Task.java Outdated
public String getTypeIcon() {
switch (type) {
case TODO:
return "T";

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 you can avoid magic strings and store the strings!

philip1304 and others added 10 commits October 9, 2024 11:56
ip // Final JAR Release
Notes:
- Added abstraction & documentation // Parser.java
- Added abstraction & documentation, fixed errors that would cause the program to exit // PlopBot.java
- Added documentation // Storage.java
- Added documentation, cleaned up code // Task.java
- Added documentation // TaskList.java
- Added documentation // Ui.java
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.

2 participants