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

Added functional lister implementation to ease lambda useage for IPC handlers #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gudenau
Copy link

@gudenau gudenau commented Dec 2, 2018

Uses a few sub-classes for the interfaces. All marked as functional with annotations.

@jagrosh
Copy link
Owner

jagrosh commented Dec 2, 2018

This looks interesting; can you provide an example of usage (showcasing its improvements over the listener interface)? Also, is there any particular reason for making the fields public instead of final private fields?

@gudenau
Copy link
Author

gudenau commented Dec 2, 2018

This model would allow you to do something like this:

IPCFunctionalListener listener = new IPCFunctionalListener();
listener.readyHandler = (client)->System.out.println("Ready");
listener.onError = this::disconnectHandler; // private void disconnectHandler(IPCClient, Throwable)
client.setListener(listener);

Having them be fields is just a "ease of use" thing. Since there would be no restrictions beyond the normal Java type system you don't need any extra setter checks. They can't be final in this model because they are not set during object creation.

In theory this might be a touch slower, but the modern JVM should optimize it enough so it does not mater as well as the infrequent use of the callbacks.

@jagrosh
Copy link
Owner

jagrosh commented Dec 2, 2018

Usually for adapters like this (and other things that activate on events) you'd want to make the fields final for safety as things get passed around (and a builder pattern for creation of the adapter).

For something that shouldn't be final, the java bean pattern should probably be used.

@gudenau
Copy link
Author

gudenau commented Dec 2, 2018

I'll change that in a couple of days.

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.

2 participants