Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add protected EventEmitter::HaveListener() function. #992

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
7 participants

lloyd commented Apr 29, 2011

if EventEmitter exposed a protected HaveListeners method then objects that emit events with payloads which are particularly costly to construct could optimize by using it. This is a naive implementation that affords a 15% performance improvement in one sample use case (yajl node bindings). This patch might be more interesting if the HaveListeners check itself could be made less expensive.

@lloyd lloyd if EventEmitter exposed a protected HaveListeners method then objects…
… that emit events with payloads which are particularly costly to construct could optimize by using it. This is a naive implementation that affords a 15% performance improvement in one sample use case. This patch would be more interesting if the HaveListeners check itself could be made less expensive
1c99451

isaacs commented Apr 29, 2011

What's wrong with emitter.listeners("foo").length === 0?

lloyd commented Apr 29, 2011

maybe nothing, it could be my ignorance. I don't understand how I access such a thing from a c++ extension, like for instance from a member of this object: https://github.com/lloyd/node-yajl/blob/master/src/handle.h#L40-61

kilianc commented Apr 30, 2011

it should be EventEmitter::HasListener()

kuebk commented May 4, 2011

@isaacs the hasListener method is more obvious.

pquerna commented Jun 13, 2011

I agree, we were talking about this last night, exposing the list of listeners as an array is wrong if we ever have things like glob listeners....

We should just hide this behind a method, its just better abstraction.

lloyd commented Jun 13, 2011

I originally called it HaveListeners() cause it's protected. it would only be invoked by a derived class on itself. But shoot, this->HasListener() is as clear to me as HaveListeners() in indicating that I'm talking about myself.

So HasListener() works. Making it public rather than protected is cool too.

bmeck commented Jan 8, 2013

@isaacs is this still relevant at all?

Owner

bnoordhuis commented Jan 8, 2013

No, EventEmitter is 100% JS now. Closing.

@bnoordhuis bnoordhuis closed this Jan 8, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment