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

com.gluonhq.attach.util.Services refactored #20

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

Conversation

DanskerDave
Copy link

No description provided.

@DanskerDave
Copy link
Author

As discussed in Mail with José Pereda

@DanskerDave
Copy link
Author

Hi José,

I've created a Fork & made 2 commits to it.
How do I make that available to you?
(as you see, I'm just as new to git as I am to Gradle) :-)

Here's what I did (& didn't) change...

Firstly, an observation:
many of the classes I looked at do not seem to take account of Multi-Threading, synchronized etc...

I have not addressed that at all.

Here are the details of my changes:

com.gluonhq.attach.util.impl.DefaultServiceFactory

Class.forName never returns null (at least, according to Javadoc)

-> superfluous null-check removed

com.gluonhq.attach.util.Services

The Javadoc of the registerServiceFactory(...) Method was incorrect.

-> Javadoc corrected

-> I added Objects.requireNonNull(factory.getServiceType()) to the Method

The current get(...) Method always does a Lookup in FACTORY_MAP,
even when the Service has already been resolved.

-> I have removed the superfluous Lookup.

Every time a Service is requested, its wrapped in a new Optional.

-> Instead of wrapping the Service instance anew every time,
I wrap it once and store the resulting Optional in the Map,
thus speeding up subsequent calls.

-> also, I've commented the Method extensively

-> the behaviour of the rewritten Method is identical to the old version.

The new Java11+ Version has unreachable code:
now that DefaultServiceFactory is no longer abstract,
ServiceFactory can never be null & the RuntimeException will never be thrown.
(instead, DefaultServiceFactory.createInstance(...) will generate a LOGGER.warning)

-> unreachable new RuntimeException(...) removed

I've also included a Dummy SERVICE_NULL_ENTRY to speed things up even more.
You need to read the Comments in the Source to decide if that's desired.

All the best,
Dave

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant