Skip to content

Adding initialization-on-demand idiom and noninstantiable class instead of interface constant idiom #532

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

Merged
merged 5 commits into from
Jan 23, 2017

Conversation

leogtzr
Copy link
Contributor

@leogtzr leogtzr commented Dec 24, 2016

Adding noninstantiable class instead of interface constant idiom.

@iluwatar iluwatar self-requested a review January 10, 2017 19:43
@iluwatar iluwatar self-assigned this Jan 10, 2017
@iluwatar
Copy link
Owner

In the Singleton example IvoryTower demonstrates eagerly initialized instance. In this pull request you have changed it implement Initialization-on-demand holder idiom. However, this has already been demonstrated in InitializingOnDemandHolderIdiom.

@iluwatar
Copy link
Owner

The second change concerning CustomerSchemaSql doesn't add any value since variables in interfaces are static and final by default.

@leogtzr
Copy link
Contributor Author

leogtzr commented Jan 11, 2017

I know they are static and final by default, the problem with the interface is that somebody can implement from it. We don't want that. Those classes were not designed to be instantiated. I am enforcing noninstantiability by using this idiom:

  • interface -> class
  • include a private constructor.

@iluwatar
Copy link
Owner

iluwatar commented Jan 11, 2017

Ok, after reading more about the constant interface idiom I tend to agree that it is better style to declare final class instead. I wonder if this idiom has been used elsewhere in the codebase? If you update the pull request and comment on this thread I can accept it.

@leogtzr
Copy link
Contributor Author

leogtzr commented Jan 19, 2017

Added a little description, thanks :)

@leogtzr
Copy link
Contributor Author

leogtzr commented Jan 21, 2017

Added a few more changes, found that several java classes were using references to implementations instead of using abstractions/interfaces. Please review :D @iluwatar

@iluwatar
Copy link
Owner

@leogtzr looks good. Still the changes concerning IvoryTower need to be reverted for the reasons I mentioned before.

Copy link
Owner

@iluwatar iluwatar left a comment

Choose a reason for hiding this comment

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

@leogtzr
Copy link
Contributor Author

leogtzr commented Jan 22, 2017

@iluwatar done, I have reverted the changes for IvoryTower.java, please take a look :)

@iluwatar iluwatar merged commit 9ec0935 into iluwatar:master Jan 23, 2017
@iluwatar
Copy link
Owner

@leogtzr thank you for the improvements!

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.

2 participants