Use own EventEmitter
#128
Use own EventEmitter
#128
Conversation
| @@ -185,7 +225,8 @@ define([ | |||
| // observer.disconnect(); | |||
| } | |||
|
|
|||
| Scribe.prototype = Object.create(EventEmitter.prototype); | |||
| var eventEmitter = EventEmitter.new(); | |||
| Scribe.prototype = Object.create(eventEmitter); | |||
There was a problem hiding this comment.
As I am using the OLOO approach for EventEmitter, I would like to be consistent so I’m thinking of (in the future) refactoring the Scribe constructor to use the same approach.
There was a problem hiding this comment.
Does that mean I'm going to have to read that goddamn long OO diatribe? :-)
There was a problem hiding this comment.
Yes! Essential reading. You only have to read it once :-P
| listener.callback.apply(null, args); | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Doesn't really belong in this file, or does it?
There was a problem hiding this comment.
Unit tests?
Also: unregister? "off", or make "on" return a handle with a dispose method, depending on convention vs ease...
There was a problem hiding this comment.
If this looks good I will move it out into a partial/module.
:-P
And they are things to add later https://github.com/guardian/scribe/pull/128/files#diff-416a38bb2d802283901df66e442a750bR53
Yes, I will write some unit tests.
|
Looks sane, nice to remove a dependency for little extra cost. See remaining comment on |
Conflicts: bower.json example.html src/scribe.js
|
@theefer Ping. |
| // TODO: unit test | ||
| // Good example of a complete(?) implementation: https://github.com/Wolfy87/EventEmitter | ||
| function EventEmitter() { | ||
| this._listeners = []; |
There was a problem hiding this comment.
Do you mean for this to be an empty object instead?
|
Worth doing a good manual test via Composer after this is merged, in case we missed something. |
|
👍 |
If you think the object construction looks strange, read this for an argument against
new,instanceof, ES6 classes, etc.If this looks good I will move it out into a partial/module.