Review of fix to eventFirer.removeListener()

[Colin Clark]

I noticed this issue when I looked at the patch as well. The extra type check is just going to slow us down. 

My first instinct is to remove the type checks in the else case altogether and go for a duckier solution. The key thing we're interested in here is the presence of the special $$guid property which identifies the listener to the event system. If it's there, remove the listener. If it's not, don't.

Colin

On 2010-06-16, at 4:38 PM, Anastasia Cheetham wrote:

> Laurel, I've just had a look at the patch you attached to
>    http://issues.fluidproject.org/browse/FLUID-3672
> 
> Your test is nice and simple, and clearly illustrates that removeListener() does not actually remove listeners.
> 
> I'm wondering about the fix: The problem is that removeListener() is only trying to remove listeners of type "object" instead of type "function." Your patch changes the condition to look for *both* "object" and "function." I'm not sure whether or not a listener of type "object" would even be reasonable, but my understanding of the event system is not quite as thorough as it could be.
> 
> I'm wondering if anyone else has a moment to look at the patch, and comment on whether my suspicion (that removeListener() need only look for "function", and not "object) is correct or not?

---
Colin Clark
Technical Lead, Fluid Project
http://fluidproject.org