7

I'm trying to build a library where you can add and remove listeners for events in a pub/sub system, but am running into an issue using method references:

// here, this::printMessage is being passed as an instance of Consumer<String>
pubSub.subscribe(this::printMessage);
pubSub.unsubscribe(this::printMessage);

Internally, calling subscribe() will add the instance of Consumer<T> to a Set<Consumer<T>>, and unsubscribe() will remove it. This issue arises from the fact that each usage of this::printMessage here actually causes the compiler to generate a new object reference/instance, so, unsubscribing doesn't actually work.

The workaround so far that I've managed is:

final Consumer<String> consumer = this::printMessage;
pubSub.subscribe(consumer);
pubSub.unsubscribe(consumer);

But, this isn't really ideal. My concern is someone less-experienced using this library may assume that they can use method references directly when subscribing/unsubscribing, when that's not really the case, and worst case, leading to a memory leak.

So the question is, is there some clever way to avoid this or coerce the method reference to always resolve to the same object reference/instance?

Gene Trog
  • 71
  • 2
  • 1
    However whoever uses `this::printMessage` should know that the compiler is generating a new object instance. It is not a problem of your library actually. – m0skit0 Jan 31 '19 at 17:42
  • 3
    There's no real way around that. You can return a `Subscription` object with an `unsubscribe()` method instead to avoid mistakes like this in the API. – Joachim Sauer Jan 31 '19 at 17:57
  • return a handle on subscription and use that to unsubscribe (of course, use that handle as the key of a map where you have all your consumers stored internally, and make that handle implement hashCode and equals consistently) – fps Jan 31 '19 at 18:01
  • 1
    Possible duplicate of [Is there a way to compare lambdas?](https://stackoverflow.com/questions/24095875/is-there-a-way-to-compare-lambdas) – Denis Zavedeev Jan 31 '19 at 19:06
  • @JoachimSauer fyi you should create an actual response to the question so the OC can mark it as the Correct Answer – barclay Jan 31 '19 at 19:35
  • If one of the answers below fixes your issue, you should accept it (click the check mark next to the appropriate answer). That does two things. It lets everyone know your issue has been resolved to your satisfaction, and it gives the person that helps you credit for the assist. [See here](http://meta.stackexchange.com/a/5235) for a full explanation. – dpr May 20 '19 at 10:38

2 Answers2

3

You could make subscribe either return the actual Consumer instance or an identifier for the added Consumer. This return value could be used in unsubscribe to remove the Consumer again.

Maybe something similar to this:

Map<UUID, Consumer<?>> consumers = new ConcurrentHashMap<>();

public UUID subscribe(Consumer<?> consumer) {
    UUID identifier = UUID.randomUUID();
    consumers.put(identifier, consumer);
    return identifier;
}

public void unsubscribe(UUID identifier) {
    consumers.remove(identifier);
}

The usage of an identifier instead of the actual Consumer instance as return value has the advantage that users of your code will directly see that they need to keep track of the returned UUID instead of using unsubscribe with a different 'identical' (in terms of behavior) Consumer.

dpr
  • 10,591
  • 3
  • 41
  • 71
  • 1
    But I wouldn’t use a `UUID` for that, as a plain object without any properties would do. – Holger Feb 18 '19 at 09:25
  • @Holger well you need a plain object with proper implementations of equals and hashcode. One could use the string representation of UUID but doing this you won’t gain anything. – dpr Feb 18 '19 at 09:28
  • 1
    Every object has an `equals` and `hashCode` implementation. The default implementation makes this object equals nothing but itself, which is precisely the desired behavior. In contrast, a `UUID` allows the construction of a different equal object via the string representation, which is *not* what you want here. You want the subscriber to keep the returned object and pass it back to `unsubscribe`, not to do transformations nor conversions on it. – Holger Feb 18 '19 at 09:31
  • @Holger actually we don't exactly know the full requirements here. Maybe the OP wants to store the reference for later removal maybe not. Yes you could use a plain object but this would unnecessarily limit the possible use cases. However the above example was only to showcase a possible solution. It was not meant as "do it this way or you did it wrong". – dpr Feb 18 '19 at 09:36
  • The OP *has* to keep a reference, whether they want or not. The possibility to convert this `UUID` to a `String` doesn’t change that. You would have to keep the string then. And the question was about a limitation of method references over ordinary objects. Ordinary listeners do not have a capability of reconstructing them via a string representation either. And there’s no sense in providing a persistent key form for a registration that doesn’t last longer than this runtime anyway. – Holger Feb 18 '19 at 09:45
0

When you write code like :

pubSub.subscribe(this::printMessage);
pubSub.unsubscribe(this::printMessage);

It is similar code like:

pubSub.subscribe(new Consumer() {

            @Override
            public void accept(Object t) {
              // your code here
            };
        });

pubSub.unsubscribe(new Consumer() {

            @Override
            public void accept(Object t) {
              // your code here
            };
        });

So from the above code, it is clear it will create a new object every time.

Now why those are similar code?

Java has introduced byte code instruction invokedynamic to construct the anonymous class and then generate byte code for lambdas/ method ref. So, In case of lambdas/method ref java generates the implementation class and generate byte code at runtime. Once the byte code generates the rest of the step is the same as a normal method call.

Is there some clever way to avoid this or coerce the method reference to always resolve to the same object reference/instance?

-> I don't believe there is any other clever way (other than what you have done as work around) to do that.

Amit Bera
  • 7,075
  • 1
  • 19
  • 42
  • Thank you, I understand why it's happening, I was just wondering if there's a way around it as it's not an obvious behavior to those coming from other languages that don't have this limitation. – Gene Trog Jan 31 '19 at 17:55
  • 1
    I don't believe there is any way around. Can you please let me know in which language it is conceptually different from Java. Thanks!!! – Amit Bera Jan 31 '19 at 17:59
  • C# is one I can think of off the top of my head: Try this: https://dotnetfiddle.net/TOIn3s – Gene Trog Feb 01 '19 at 15:33