Skip to content

implements Twin design pattern #63#290

Merged
iluwatar merged 2 commits intoiluwatar:masterfrom
hoswey:master
Nov 16, 2015
Merged

implements Twin design pattern #63#290
iluwatar merged 2 commits intoiluwatar:masterfrom
hoswey:master

Conversation

@hoswey
Copy link
Copy Markdown
Contributor

@hoswey hoswey commented Nov 14, 2015

please help review.

@hoswey
Copy link
Copy Markdown
Contributor Author

hoswey commented Nov 14, 2015

@iluwatar, the converages check is so strange as I find the check of my source at lease reach to 86.36.

alt text

@iluwatar
Copy link
Copy Markdown
Owner

  • Put under review badge to the pull request
  • Does the example code implement the pattern correctly and follow good coding practices?
  • Does the example code have enough test coverage?
  • Is the example code commented well enough?
  • Is the example code following JavaDoc conventions?
  • Are the project coding conventions being followed?
  • Is the class diagram generated correctly?
  • Is the index.md implemented correctly so the pattern will show correctly on the web site?
  • Is the originator of the design pattern properly credited?

Here are my remarks:

  • The pattern explanation in App.java could be a bit more verbose and the typos and phrasing should be fixed
  • The originator of the design pattern should be properly credited in index.md. See the pattern template

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Multiple typos and unclear text. This should be rephrased.

@iluwatar
Copy link
Copy Markdown
Owner

Overall the implementation is well done and only minor issues were found. Please comment on this thread when you've implemented the changes and I will review again.

@hoswey
Copy link
Copy Markdown
Contributor Author

hoswey commented Nov 16, 2015

Thanks @iluwatar 's comments. Fix is done, please review.

iluwatar added a commit that referenced this pull request Nov 16, 2015
implements Twin design pattern #63
@iluwatar iluwatar merged commit ba3f583 into iluwatar:master Nov 16, 2015
@iluwatar
Copy link
Copy Markdown
Owner

Thank you @hoswey Well done!

@npathai
Copy link
Copy Markdown
Contributor

npathai commented Nov 17, 2015

Thanks for your contribution @hoswey

pratigya0 pushed a commit to pratigya0/java-design-patterns that referenced this pull request Aug 3, 2023
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.

3 participants