patternMinor
Do I need to remove these event listeners?
Viewed 0 times
theseneedremovelistenersevent
Problem
The following ActionScript code just downloads and attempts to parse some JSON from an API service. Inside the method I have defined a
I think these event listeners will be garbage collected anyway though, so is it worth it? I would prefer to just use 2 anonymous functions for the listeners.
cleanUp() function, just to remove 2 event listeners.I think these event listeners will be garbage collected anyway though, so is it worth it? I would prefer to just use 2 anonymous functions for the listeners.
private function sendRequest(cmd:String, params:Object = null) :Promise{
var d:Deferred = new Deferred(),
vars:URLVariables = new URLVariables('cmd=' + cmd),
req:URLRequest = new URLRequest(),
loader:URLLoader = new URLLoader();
if(params){
for(var paramKey:String in params){
vars[paramKey] = params[paramKey];
}
}
req.url = url;
req.requestHeaders = [new URLRequestHeader('Accept', 'application/json')];
req.method = URLRequestMethod.POST;
req.data = vars;
function onComplete(e:Event) :void{
var res:Object;
try{
d.resolve(JSON.parse(e.target.data));
}
catch(err:Error){
d.reject(err);
}
finally{
cleanUp();
}
}
function onError(err) :void{
d.reject(err);
cleanUp();
}
function cleanUp() :void{
loader.removeEventListener(Event.COMPLETE, onComplete);
loader.removeEventListener(IOErrorEvent.IO_ERROR, onComplete);
};
loader.addEventListener(Event.COMPLETE, onComplete);
loader.addEventListener(IOErrorEvent.IO_ERROR, onError);
loader.load(req);
return d.promise;
};Solution
Quick answer:
Maybe yes, maybe no. Yes, they'll be garbage collected now, but cleaning up after yourself is not a bad habit to get into. It doesn't hurt to do so.
What you do need to be careful of, however, is that you clean it up right.
Compare:
With:
and you'll soon notice the mistake:
Additionally, you have a unnecessary variable here:
It's not used by anything, so I'd remove it.
Maybe yes, maybe no. Yes, they'll be garbage collected now, but cleaning up after yourself is not a bad habit to get into. It doesn't hurt to do so.
What you do need to be careful of, however, is that you clean it up right.
Compare:
loader.addEventListener(Event.COMPLETE, onComplete);
loader.addEventListener(IOErrorEvent.IO_ERROR, onError);With:
loader.removeEventListener(Event.COMPLETE, onComplete);
loader.removeEventListener(IOErrorEvent.IO_ERROR, onComplete);and you'll soon notice the mistake:
IOErrorEvent.IO_ERROR is mapped to onError, but the removeEventListener assumes it's mapped to onComplete!Additionally, you have a unnecessary variable here:
var res:Object;It's not used by anything, so I'd remove it.
Code Snippets
loader.addEventListener(Event.COMPLETE, onComplete);
loader.addEventListener(IOErrorEvent.IO_ERROR, onError);loader.removeEventListener(Event.COMPLETE, onComplete);
loader.removeEventListener(IOErrorEvent.IO_ERROR, onComplete);var res:Object;Context
StackExchange Code Review Q#52651, answer score: 2
Revisions (0)
No revisions yet.