Java enumeration that is registered as a listener when created

There are two good (as most believe) java practices that I am trying to combine and fail.

  • Don't miss this in the constructor.
  • Use enumeration instead of singleton pattern.

So, I want a singleton that as soon as it is created listens for some event. Here's an example. First, the event listener interface:

public interface EventListener {
    void doSomething();
}

      

Then the event creator:

public class EventProducer implements Runnable{

    private EventListener listener;

    public EventProducer(EventListener listener) {
        if (listener == null) {
            throw new NullPointerException("Listener should not be null.");
        }
        this.listener = listener;
    }

    @Override
    public void run() {
        listener.doSomething(); //This may run before the listener is initialized.
        do {
            long startTime = System.currentTimeMillis();
            long currentTime;
            do {
                currentTime = System.currentTimeMillis();
            } while ((currentTime - startTime) < 1000);
            listener.doSomething();
        } while (!Thread.currentThread().isInterrupted());
        listener = null; //Release the reference so the listener may be GCed
    }
}

      

Then an enum (as the 2nd of the listed java practices shows):

public enum ListenerEnum implements EventListener{

    INSTANCE;

    private int counter;
    private final ExecutorService exec;

    private ListenerEnum() {
        EventProducer ep = new EventProducer(this); //Automatically unregisters when the producer is done.
        counter = 0;
        exec = Executors.newSingleThreadExecutor();
        exec.submit(ep);
    }

    @Override
    public void doSomething() {
        System.out.println("Did something.");
        counter++;
        if (counter >= 5) {
            exec.shutdownNow();
        }
    }
}

      

And finally, start something:

public class TestRunner {

    public static void main(String[] args) {
        ListenerEnum.INSTANCE.doSomething();
    }

}

      

The problem is with the first line of the ListenerEnum constructor as we are leaking this, which is not in line with the 1st listed java practice. This is why our event producer can call the listener method before the listener is created.

How can I handle this? Normally I would use the Builder pattern, but how is this possible with an enum?

EDIT: For those of you who matter, the event producer in my program actually extends the BroadcastReceiver, so my enum cannot be the event producer, they must be separate. The manufacturer is created in the constructor of the enum (as an example) and later registered programmatically. So I have no problem with that. However, I would like to know if I can avoid this.

EDIT 2:

Ok, since there are suggestions for solving my problem, I would like to clarify a few things. First of all, most of the suggestions are workarounds. They suggest doing the same thing in a completely different way. I appreciate these suggestions and will probably agree with them as an answer and implement it. But the real question should be "How do I implement an enum Builder pattern?" The answer I already know, and people think that "you don't do it, do it differently." Is there anyone who can post something like "You do it! You do it like this".

I was asked to give a code close to my actual use case. Change the following:

public enum ListenerEnum implements EventListener{

    INSTANCE;

    private EventProducer ep;
    private int counter;
    private ExecutorService exec;

    private ListenerEnum() {
        ep = new EventProducer(this); //Automatically unregisters when the producer is done.
        counter = 0;
    }

    public void startGettingEvents() {
        exec = Executors.newSingleThreadExecutor();
        exec.submit(ep);
    }

    public void stopGettingEvents() {
        exec.shutdownNow();
    }

    @Override
    public void doSomething() {
        System.out.println("Did something.");
        counter++;
        if (counter >= 5) {
            stopGettingEvents();
        }
    }
}

      

Besides:

public class TestRunner {

    public static void main(String[] args) {
        ListenerEnum.INSTANCE.startGettingEvents();
    }

}

      

Now all I have to do to solve my problem is move the creation of EventsProducer to the startGettingEvents () method. It. But this is also a workaround. What I would like to know is this: In general, how to avoid leaking this in the listener enum constructor since you cannot use the Builder pattern? Or can you somehow use the Builder enum pattern? Is this done only in workarounds on a case-by-case basis? Or is there a general way to handle this that I am not aware of?

+3


source to share


3 answers


Just create a static initialization block:

public enum ListenerEnum implements EventListener{

    INSTANCE;

    private int counter;
    private static final ExecutorService exec; //this looks strange. I'd move this service out of enum.
    private static final EventProducer ep;

    static{
            exec = Executors.newSingleThreadExecutor();
            ep = new EventProducer(INSTANCE); //Automatically unregisters when the producer is done.
            exec.submit(ep); 
    }

    @Override
    public void doSomething() {
        System.out.println("Did something.");
        counter++;
        if (counter >= 5) {
            exec.shutdownNow();
        }
    }
}

      



As long as the enumeration values ​​are final and static, they are initialized before the static initialization block. If you decompile the enum, you will see one initialization block:

        static{
                INSTANCE = new ListenerEnum();
                exec.submit(INSTANCE.ep); 
        }

      

+3


source


First, think about why this

you shouldn't run:

  • You will void the final

    Field Publishing Guarantee if the copy is incorrectly published
  • Even with safe publishing, there are inconsistencies regarding all actions not performed in the constructor at the time of the leak.
  • You will be able to avoid incomplete instance in case of subclasses, since the subclass's constructor has not been called yet

This does not apply to you in this narrow case. Submitting to is Executor

not a mis-post and enum

s can not escape in any other way than the one you implemented yourself in the constructor. And the last in the constructor, while it enum

has no subclasses.


Now that you've edited your question, it matters a lot less. Constructor

private ListenerEnum() {
    ep = new EventProducer(this);
    counter = 0;
}

      



is not a "leak this

" unless ep

it is a variable static

, and the constructor EventProducer

does not leak it either this

. This is important because programmers should be able to create pie charts of objects without fear of sudden inconsistencies.

But you still don't have to do anything too easily. As said, it relies on behavior EventProducer

regarding leakage and what EventProducer

should not be returned to ListenerEnum

, which could disrupt performance without this

technically "leaking ". Ultimately, you can create code that breaks without compromising thread safety.

So its a code for which you cannot see the correctness by looking at it as you need knowledge of another class.

There are cases where transferring this

to another object is considered safe due to well-known behavior, for example. weakThis=new WeakReference(this);

- a real example. However, transferring this

to something called EventProducer

is likely to result in wake-up calls for every reader, which you should avoid, even if you know for sure it's a false alarm.


The big design smell, however, is the use of the Singleton pattern by itself. After all, every instance you create is unique in the first place. The peculiarity of the Singleton pattern is that it provides global access public

to this instance. Is this really what you want? Do you think that with the Singleton pattern, everyone within the entire application can register this listener again?

+2


source


The fact that your class is singleton (whether enum based or otherwise) is not related to your problem. Your problem is how to register the listener inside the object's constructor. And the answer is: it is impossible, safe.

I would recommend that you do two things:

  • Make sure your listener is not missing events by having a queue that it polled for work. Thus, if it is temporarily not bugged, the work simply stands in line. What this really means is that you don't really need to be a listener in the traditional sense. He just needs to poll the queue.

  • Register the class as a listener using a separate method as described in the comments.

I would think to avoid the singleton. It doesn't offer many benefits (apart from the minor benefit of being able to call SomeClass.INSTANCE

from anywhere). The disadvantages are most noticeable during testing, where it is much more difficult for you to mock the class when you want to test without actually sending things over the network.


Here's a concrete example of why the leak this

is dangerous in your case. Your constructor goes through this

before setting the counter to zero:

private ListenerEnum() {
    ep = new EventProducer(this);
    counter = 0;
}

      

Now, once it this

accelerates, the event producer can call doSomething()

5 times before the constructor completes:

@Override
public void doSomething() {
    System.out.println("Did something.");
    counter++;
    if (counter >= 5) {
        exec.shutdownNow();
    }
}

      

Should the sixth call to this method fail? Except that your constructor ends up and sets counter = 0;

. This allows the producer to call doSomething()

5 more times.

Note: It doesn't matter if you change these lines, as the constructor cannot be executed in the order in which it appears in your code.

+1


source







All Articles