Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[W7][T11-2]Matthias Lum #47

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

matthiaslum
Copy link

@matthiaslum matthiaslum commented Mar 4, 2019

Added an appointment class so that a user can input an appointment date with a doctor.

Copy link

@stephlewyh stephlewyh left a comment

Choose a reason for hiding this comment

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

Hi Matthias,

Excellent effort. You have updated the documentation, furnished your enhancement and considered adding tests.

One thing to note: your default example value in the Appointment class is 30November. In your test case input, it is also 30November. You could have supplied a different input, to show that your function is able to take in other appointment date values.

@@ -60,6 +62,8 @@ public ReadOnlyPerson getPerson() {
public CommandResult execute() {

Choose a reason for hiding this comment

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

Missing header comment. All non-trivial methods should have java doc format header comments.

return appointmentDate;
}

@Override

Choose a reason for hiding this comment

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

Missing header comment. All non-trivial methods should have java doc format header comments.

@@ -30,6 +31,7 @@ default boolean isSameStateAs(ReadOnlyPerson other) {
&& other.getName().equals(this.getName()) // state checks here onwards
&& other.getPhone().equals(this.getPhone())
&& other.getEmail().equals(this.getEmail())
// && other.getAppointment().equals(this.getAppointment())

Choose a reason for hiding this comment

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

Did you intend to comment this? Note that irrelevant comments added by you should be removed in your submissions.

@stephlewyh
Copy link

Please close this PR after reviewing.

@stephlewyh stephlewyh closed this Mar 17, 2019
@stephlewyh stephlewyh reopened this Mar 17, 2019
yingrong1996 added a commit to yingrong1996/addressbook-level3 that referenced this pull request Mar 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants