What's the better option for anonymous event listeners?

Question

I'm trying to figure out the best option to use anonymous event listeners that are 100% garbage collected after use. I got these two examples but I'm wondering if it actually makes any difference between them ...

var listener1:Function = function(e:Event):void
{
 resource.removeEventListener(e.type, listener1);
 loadedHandler(resource);
 listener1 = null;
};
resource.addEventListener(ResourceEvent.LOADED, listener1);

or this one ...

resource.addEventListener(ResourceEvent.LOADED, function(e:Event):void
{
 Resource(e.currentTarget).removeEventListener( e.type, arguments["callee"]);
 loadedHandler(resource);

}, false, 0, true);

Or would there be any other, better solution? It's very important for me that these listeners and functions get removed correctly from memory because they might be executed very often in the application. I could go and use a Dictionary to map all listeners etc. and test and remove them later in non-anonymous listeners etc. etc. but that would get very involved quickly because there might be situations where resources can be loaded asynchronously at the same time at different classes in the app.

---

@dominic: You mean placing the function like that into the method body, right? As I wrote above the app loads resources asynchronous and it could happen that a resource is currently being loaded while another class from somewhere else in the app requests the same resource. The resource managing class (in which the said code is) then hooks up to the event listeners dispatched by a resource. As far as I understand this if I use class methods or functions like in your example as listeners they will be re-used by a new request and the events for the older request will never fire. Hence the anonymous functions stored in a variable. I assume they all stay in memory until their respective request is done. But maybe I totally confuse this and it's not the case? I sometimes find the event stuff very hard to grasp.

Answer 1

The first one looks cleaner, and you avoid using a cast and an array lookup to remove the listener. Why are you using "anonymous" handler functions ? You could use it like this, it's a little cleaner, and that's the way the asdocs suggest the event model be used.

function listener1(e:ResourceEvent):void
{
    resource.removeEventListener(e.type, listener1);
    loadedHandler(resource);
};
resource.addEventListener(ResourceEvent.LOADED, listener1);

Answer 2

This does not answer your "vs" question directly, but I am about to release¹ an AS3 port of the .NET Reactive Extensions framework called RxAs which solves problems just like this:

Observable.fromEvent(resource, ResourceEvent.LOADED)
    .take(1)
    .subscribe(function(e : ResourceEvent) : void
    {
        // just the subscriber code
    });

The use of take means that it will auto-unsubscribe after the first event. You can also hand subscribe a reference to a method, if you don't like anonymous functions, without worrying about removing it later.

While it's not intended to be, please let me know if this answer is too 'spammy' and I'll remove it.

¹ There'll be a binary release within the week but you can download the source now

Answer 3

I see no advantage in using anonymous listeners in this case (could be cleaner in some other scenarios, though, but I think you gain nothing here).

Anyway, there are a couple of things to note:

1) Your second option is asking for trouble. Dont' use weak references for anonymous functions. Your listener could be collected before the event is dispatched. (I'd say don't use weak references period, but in most cases is rather a personal taste; in this case, however, it isn't).

2) This approach will work as long as you can be sure that the listener will always fire and that that's the only event that will be dispatched. If your dispatcher fires some event to signal an error, your listeners will not be removed.

3) Removing a listener is not necessarily related to garbage collection. In some cases, not removing a listener will cause a leak (when listening on the stage, for instance). In most cases, though, not removing a listener will not cause a leak (even with strong refs). I understand that you might want to remove the listeners because you no longer want to listen for some event, and that is correct, but I though I'd just add this point.

Answer 4

The event handler function / method isn't really the probleme here.

If I understand what your saying, is that you have a ResourceManager class that loads a certain resource when requested to do so by a class in your app. When the resource is fully loaded, the resource dispatches an event that will trigger your event handler:

resource.addEventListener(ResourceEvent.LOADED, listener1);

So now the listener1 function or method will be called. The thing is, if in listener1 you decide to remove the event listener, now further load completion of resources will trigger the listener1 handler, because the manager is no longer listening for it. And if two different classes in the app load resources at the same time, the listener1 handler will be called once for each ResourceEvent.LOADED event.

In my humble opinion, you should leave the event listener, and remove it only once all resources have been loaded, and use the manager to control access to resource loading, so that it is centralized, and all ResourceEvent.LOADED event will be handled by the listener1 function / method. Then of cource, if your app loads resource during the entire of its life time, don't remove that listener, only remove it once when you don't need it anymore.

I'm not 100% sure I understood what you meant so I hope I'm not totaly of subject here! Hope this helps.

Answer 5

Thanks for the hint all! I've went with a slightly more involved solution now by creating a simple map to that the load and fail handlers are mapped by the resource ID. Here's the basic idea:

This replaces my original code:

private var _listeners:Object = {};

public function load(resourceID:String):void
{
    ...

    if (loadedHandler != null || failedHandler != null)
    {
        _listeners[resource.id] = new ListenerVO(loadedHandler, failedHandler);
        if (loadedHandler != null)
        {
            resource.addEventListener(ResourceEvent.LOADED, onResourceProcessed);
        }
        if (failedHandler != null)
        {
            resource.addEventListener(ResourceEvent.FAILED, onResourceProcessed);
        }
    }

    ...
}

Here's the new event handler:

private function onResourceProcessed(e:ResourceEvent):void 
{
    var r:Resource = e.resource;
    var handler:Function;
    r.removeEventListener(e.type, onResourceProcessed);

    if (e.type == ResourceEvent.LOADED)
        handler = ListenerVO(_listeners[r.id]).loadedHandler;
    else if (e.type == ResourceEvent.FAILED)
        handler = ListenerVO(_listeners[r.id]).failedHandler;

    if (handler != null)
    {
        _listeners[r.id] = null;
        delete _listeners[r.id];
        handler(r);
    }
}

And a small package-Level VO for keeping it all strong-typed ...

class ListenerVO
{
    public var loadedHandler:Function;
    public var failedHandler:Function;

    public function ListenerVO(lh:Function, fh:Function)
    {
        loadedHandler = lh;
        failedHandler = fh;
    }
}

Let me know what you think of this idea!