Skip to content

[Caleb Chan] iP#191

Open
calebcjl wants to merge 26 commits into
nus-cs2113-AY2223S2:masterfrom
calebcjl:master
Open

[Caleb Chan] iP#191
calebcjl wants to merge 26 commits into
nus-cs2113-AY2223S2:masterfrom
calebcjl:master

Conversation

@calebcjl

Copy link
Copy Markdown

No description provided.

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

Mostly clean code that is easy to follow. Minor amendments to naming and cosmetics requested.

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

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

Overall good job following the java coding standards. Variables names are in camelCase, constant names are in uppercase with underscore to separate the words. Method names are verbs.

Comment thread src/main/java/Nano.java Outdated
Comment thread src/main/java/Nano.java Outdated
Comment thread src/main/java/Nano.java Outdated
Comment thread src/main/java/Task.java
Comment thread src/main/java/Nano.java Outdated
Comment thread src/main/java/Nano.java Outdated
New classes are:
1. Deadline - Task with due date
2. Event - Task with start and end date
3. Todo - regular Task
Exceptions added:
1.  DukeCommandExeception: user entered invalid command
2.  DukeInputFormatException: user input has wrong format

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

Great! Very concise and readable code. No coding violation found

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