Skip to content

[W5][T11-1]Sam - #55

Open
Sameow wants to merge 1 commit into
nusCS2113-AY1819S2:masterfrom
Sameow:W5
Open

[W5][T11-1]Sam#55
Sameow wants to merge 1 commit into
nusCS2113-AY1819S2:masterfrom
Sameow:W5

Conversation

@Sameow

@Sameow Sameow commented Feb 11, 2019

Copy link
Copy Markdown

Encapsulated a variable

@JKupoluyi

Copy link
Copy Markdown

I like what you did there

@Shizhe-NUS

Copy link
Copy Markdown

Nice coding! I wish I can code like you!

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

Hi Samuel,

You have attempted an OOP enhancement via a getter method and updated the user documentation, albeit in a non-standard way.

However, you need to update your header comment for code readability. Also, you could have provided a JUnit test case for getFeedbackToUser() to verify if it returns the expected string.

Nevertheless, not bad for a first attempt!

return Optional.ofNullable(relevantPersons);
}

public String getFeedbackToUser() {

Copy link
Copy Markdown

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.

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.

5 participants