X Tutup
Skip to content

MonoState Pattern#258

Merged
iluwatar merged 9 commits intoiluwatar:masterfrom
amit2103:master
Oct 13, 2015
Merged

MonoState Pattern#258
iluwatar merged 9 commits intoiluwatar:masterfrom
amit2103:master

Conversation

@amit2103
Copy link
Copy Markdown
Contributor

@amit2103 amit2103 commented Oct 3, 2015

Implemented the pattern. Created a custom load balancer. Will add java comments.
The Documentation and Class Diagram is present. Please let me know if any other things needs to be done.

@amit2103
Copy link
Copy Markdown
Contributor Author

amit2103 commented Oct 5, 2015

I will add the Junits, but I would like to now whether the design is okay

@iluwatar
Copy link
Copy Markdown
Owner

iluwatar commented Oct 6, 2015

The initial code looks ok. The branch is not up to date with master. Please catch up the branch and implement the missing items in the following checklist.

  • 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?

@npathai
Copy link
Copy Markdown
Contributor

npathai commented Oct 7, 2015

I am also reviewing it.

@amit2103
Copy link
Copy Markdown
Contributor Author

amit2103 commented Oct 7, 2015

@iluwatar Thanks for the comment. Will get it done by this weekend.
@npathai Let me know if you have any extra comments. i will incorporate them

@amit2103
Copy link
Copy Markdown
Contributor Author

amit2103 commented Oct 7, 2015

Don't know what caused the build break. I have updated the files with comments and Junits. Locally my build passed.
Please do a recheck when it is possible.

@iluwatar
Copy link
Copy Markdown
Owner

iluwatar commented Oct 7, 2015

[ERROR] Non-resolvable parent POM: Failure to find com.iluwatar:java-design-patterns:pom:1.6.0 in http://repo.maven.apache.org/maven2 was cached in the local repository, resolution will not be reattempted until the update interval of central has elapsed or updates are forced and 'parent.relativePath' points at wrong local POM @ line 5, column 11 -> [Help 2]

Use version number 1.7.0 in pom.xml to resolve this.

Fixed Reference to Parent POM
@amit2103
Copy link
Copy Markdown
Contributor Author

amit2103 commented Oct 8, 2015

@iluwatar Thanks. Understood. I had already done a build with the parent POM at 1.6.0. So when I did a local build yesterday, it passed since my local maven repo had a reference to 1.6.0.

I have corrected the reference to the parent POM. Can you review the code?

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.

should be "Singleton"

@iluwatar
Copy link
Copy Markdown
Owner

iluwatar commented Oct 9, 2015

@npathai Any additional comments?

@amit2103
Copy link
Copy Markdown
Contributor Author

amit2103 commented Oct 9, 2015

@iluwatar I noticed the pom error was about to commit the changes with one minor modification. Which would improve Junit coverage. Committed the changes. Please do a review once you have time.

@npathai
Copy link
Copy Markdown
Contributor

npathai commented Oct 12, 2015

@iluwatar Had been away for some days. Will review it today.

@npathai
Copy link
Copy Markdown
Contributor

npathai commented Oct 13, 2015

Good work. Looks good to go. Thanks for your contribution.

@amit2103
Copy link
Copy Markdown
Contributor Author

@npathai Thanks...Will start working on the leader follower pattern

iluwatar added a commit that referenced this pull request Oct 13, 2015
@iluwatar iluwatar merged commit 3c8b837 into iluwatar:master Oct 13, 2015
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

X Tutup