Skip to content

[Clarence Chua Ying How] iP#84

Open
cheshire-doge wants to merge 27 commits into
nus-cs2113-AY2122S2:masterfrom
cheshire-doge:master
Open

[Clarence Chua Ying How] iP#84
cheshire-doge wants to merge 27 commits into
nus-cs2113-AY2122S2:masterfrom
cheshire-doge:master

Conversation

@cheshire-doge

Copy link
Copy Markdown

No description provided.

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

Good effort! More comments within your code will make it easier to follow. You may want to consider methods to reduce "magic numbers".

Comment thread src/main/java/Deadlines.java Outdated

@Override
public String toString() {
return String.format("[D]%s (by: %s)", super.toString(), this.deadline);

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 like how you used the String format method to do this - this looks much cleaner than what I did!

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

} else if (input.contains("unmark ")) {

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
} else if (input.contains("unmark ")) {
} else if (input.startsWith("unmark ")) {

Could startsWith be more appropriate?

Comment thread src/main/java/Task.java Outdated

public String description;
public Boolean isDone = false;
public String mark = " ";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this still necessary given that you already have isDone?

Comment thread src/main/java/duke/Duke.java Outdated
private static Scanner sc = new Scanner(System.in);
private static Task[] list = new Task[100];
private static int taskCounter = 0;
private static String HORIZONTAL_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.

Consider adding the final modifier to the string if it is a constant.

Comment thread src/main/java/duke/Duke.java Outdated
Comment on lines +2 to +3
import duke.task.*;
import duke.exception.*;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Import classes explicitly instead of using Wildcard (*), to comply with coding standards.

Comment thread src/main/java/duke/Duke.java Outdated
public class Duke {

private static Scanner sc = new Scanner(System.in);
private static Task[] list = new Task[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.

Consider renaming "list" to make it clearer on what the variable is storing. Perhaps "tasks" would be more suitable?

Comment thread src/main/java/duke/Duke.java Outdated
Comment on lines +42 to +49
} else if (input.contains("unmark")) {
handleUnmark(input);

} else if (input.contains("mark")) {
handleMark(input);

} else if (input.contains("todo")) {
handleToDo(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.

Consider refactoring magic strings such as "unmark", "mark" into constants.

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

private static void handleUnmark(String input) throws AlreadyUnmarkedException, InvalidNumberException {
int markInt = Integer.parseInt(input.substring(7)) - 1;
if (markInt + 1> taskCounter) {

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 standardize spaces before and after operators e.g markInt + 1 > taskCounter

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