Skip to content

[Anderson Leong] iP#206

Open
uosjapuelks wants to merge 50 commits intonus-cs2113-AY2122S1:masterfrom
uosjapuelks:master
Open

[Anderson Leong] iP#206
uosjapuelks wants to merge 50 commits intonus-cs2113-AY2122S1:masterfrom
uosjapuelks:master

Conversation

@uosjapuelks
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@Ashley-Lau Ashley-Lau left a comment

Choose a reason for hiding this comment

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

Overall, good effort in the checks for the input. However, do make sure not to put too much code into one line!

Comment thread src/main/java/duke/Duke.java Outdated
return userInput;
}

public static String GetItem(String userInput) throws IllegalToDoException, InvalidCommandException, EmptyCommand {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Follow camelCase naming convention for methods


public class Event extends Task {

protected String at;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Private may be a better access modifier since no class extends Event.


public class Event extends Task {

protected String at;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

naming for at could be more descriptive of what it represents

Comment thread src/main/java/duke/Duke.java Outdated
private static void AcknowledgeAddition(String command, ArrayList<Task> unfilteredTasks) {
if (isToDo(command) || isDeadline(command) || isEvent(command)) {
System.out.println(LINE + "Got it. I've added this task:\t");
System.out.println(String.format("\t%d.", unfilteredTasks.size()) + unfilteredTasks.get(unfilteredTasks.size() - 1) + "\n" + String.format("\tNow you have %d tasks in the list.\n", unfilteredTasks.size()) + 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.

Too many characters on this line! Occurred on other lines in this code

Comment thread src/main/java/duke/Duke.java Outdated
unfilteredTasks = loadFile(FILE_PATH, unfilteredTasks);
} catch (FileNotFoundException e) {
File dukeCheckpoint = new File(FILE_PATH);
try {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Instead of nesting try-catch block, try to include then in the same level.
try {
...
} catch (FileNotFoundException e) {
...
} catch (FileNotFoundException e){
...
}

Comment thread src/main/java/duke/Duke.java Outdated
+ "What do you need to do?\n"
+ LINE;

public static String getCommand(String userInput) {
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 are some methods private and some public? Seems to me that they are all only used in this class

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