mondora / asteroid

An alternative client for a Meteor backend
MIT License
734 stars 101 forks source link

Implement once method for EventEmitter #62

Closed trever closed 8 years ago

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.94%) to 69.62% when pulling 4cc85672349322df4d7335fe53b66de6e29cb0ec on trever:pr_trever into e9f1cca63a900b31125f31fde0ff6d41630eafc3 on mondora:master.

pscanf commented 9 years ago

Hey @trever,

first of all, awesome, thanks for the PR! :smiley:

If you don't mind, before I merge I'd ask you to review a couple of things in the PR:

  1. the files you modified are the artifacts generated after building the library running gulp build (they are commited to allow getting Asteroid via bower). You should instead make changes to the source file for the Asteroid.utils.EventEmitter class, and then rebuild the library
  2. by doing delete self._events[name] when the handler is called, all other handlers for the event are de-registered, which might be an unwanted behaviour. I would suggest doing something like
once: function (name, handler) {
    var self = this;
    if (!this._events) this._events = {};
    this._events[name] = this._events[name] || [];
    var handlerWrapperIndex;
    var handlerWrapper = function () {
        var args = Array.prototype.slice.call(arguments);
        // Remove only the previously inserted handlerWrapper
        self._events[name].splice(handlerWrapperIndex, 1);
        handler.apply(self, args);
    };
    // `.push` returns the length of the array after the element insertion.
    // Therefore the index at which the element is inserted is the value
    // returned by `.push` minus 1
    handlerWrapperIndex = this._events[name].push(handlerWrapper) - 1;
}

It's off the top of my hat, I haven't actually tested it, so it might not work. Btw, if you could add a couple of unit tests for the method it'd be awesome (to tell the truth, the whole class needs testing, so if you want to give it a go I won't stop you :smile: )

Thanks again and hear you soon pscanf

trever commented 9 years ago

Totally! Been working with the compiled files and totally spaced on the fact that they came from bower!

I'll do some testing + write some test and resubmit a PR.

Thanks for building this! Really awesome stuff!

pscanf commented 9 years ago

Awesome, thanks! :smiley:

pscanf commented 8 years ago

In 2.0.0 Asteroid uses wolfy87-eventemitter, which implements the once method.