0

I ma trying to have my custom javascript objects be able to dispatch thier own events. I have this code exmaple. The problem is that events are duplicated when I create more than 1 object. Why is this hapenning?

(function() {
  var EventDispatcher = function() {

    var self = this;

    self.events = {};

    self.addEventListener = function(name, handler) {
      if (self.events.hasOwnProperty(name)) {
        self.events[name].push(handler);
      } else {
        self.events[name] = [handler];
      }
    };

    self.removeEventListener = function(name, handler) {
      if (!self.events.hasOwnProperty(name)) return;

      var index = self.events[name].indexOf(handler);
      if (index != -1) self.events[name].splice(index, 1);
    };

    self.fireEvent = function(name, args) {
      if (!self.events.hasOwnProperty(name)) return;

      if (!args || !args.length) args = [];

      var evs = self.events[name],
        l = evs.length;
      for (var i = 0; i < l; i++) {
        evs[i].apply(null, args);
      }
    };

  };

  window.EventDispatcher = EventDispatcher;
}(window));


(function(window) {
  "use strict"
  var PlaylistManager = function(data) {

    var self = this

    this.test = function() {
      self.fireEvent('PlaylistManager.COUNTER_READY', [{
        counter: "a"
      }]);
    }

  }

  PlaylistManager.prototype = new EventDispatcher();

  window.PlaylistManager = PlaylistManager;

}(window));


var _PlaylistManager = new PlaylistManager();

_PlaylistManager.addEventListener('PlaylistManager.COUNTER_READY', function(e) {
  console.log('COUNTER_READY')

})


var _PlaylistManager2 = new PlaylistManager();

_PlaylistManager2.addEventListener('PlaylistManager.COUNTER_READY', function(e) {
  console.log('COUNTER_READY 2')

})

_PlaylistManager.test()

_PlaylistManager2.test()
Peter Seliger
  • 11,747
  • 3
  • 28
  • 37
Toniq
  • 4,492
  • 12
  • 50
  • 109
  • You're doing the inheritance wrong. Where did you learn this `PlaylistManager.prototype = new EventDispatcher();` thing? – Bergi Oct 16 '22 at 19:44
  • What happens when the OP does ... `PlaylistManager.prototype = new EventDispatcher();` ... is, that from this very point every `PlaylistManager` instance refers to (shares) the same sole `EventDispatcher` instance. Thus the `EventDispatcher` implementation should be renamed to `Observable` or `EventTarget` and then be **applied as _function based mixin_** via e.g. ... `const plm = new PlaylistManager(); EventTarget.call(plm);` ... to every newly created `PlaylistManager` instance. – Peter Seliger Oct 16 '22 at 19:51
  • 1
    @PeterSeliger Yeah, that's another way to do it, not to use prototype inheritance at all. – Bergi Oct 16 '22 at 19:52
  • @PeterSeliger - can you make an example? – Toniq Oct 17 '22 at 08:32
  • @Toniq ... the OP needs to get rid of following line ... `PlaylistManager.prototype = new EventDispatcher();`. `EventDispatcher` now is just a function which looks like a constructor. But it does not get used as such; instead it has to be applied to any object like this ... `EventDispatcher.call(anyObject)` ... where `anyObject` afterwards features the methods of an event target like `addEventListener`, `removeEventListener` and `fireEvent` which the OP might consider renaming to `dispatchEvent` and also make capable of throwing kind of an event object instead of the `args` array. – Peter Seliger Oct 17 '22 at 09:38
  • @PeterSeliger - I am the OP, thats why I am asking if you can provide full example so Ican understand better? – Toniq Oct 17 '22 at 21:37
  • @Toniq ... I know you're the OP. Therefore the above comment was addressed to you. Its content was as much as one could go with the character count limit. Everything is included. Another answer is not possible due to the closed question/thread. – Peter Seliger Oct 17 '22 at 22:20
  • @PeterSeliger Can you use jsfiddle? – Toniq Oct 18 '22 at 08:44
  • 1
    @Toniq ... I literally can not. Btw, just do as advised (it's 2 tiny changes) and things will go well. If not, come up with a new question about the function based event target mixin and why it does not work (or where you got stuck). – Peter Seliger Oct 18 '22 at 09:07
  • @PeterSeliger - I have done this: https://jsfiddle.net/ng50cw6b/2/ Please tell me if its correct now and if you can add this part? "capable of throwing kind of an event object instead of the args array" – Toniq Oct 18 '22 at 13:04
  • @Toniq ... There is a new answer to another question of yours which you did ask two-and-a-half weeks ago ... [How to implement an own event dispatching system for ES/JS object types?](https://stackoverflow.com/questions/73894457/how-to-implement-an-own-event-dispatching-system-for-es-js-object-types/74140087#74140087) – Peter Seliger Oct 20 '22 at 13:00
  • Thanks but this is way too complicated for me. I realized dont really need any more functionality except those in the above fiddle I left. I am only not sure why this answer contains setPrototypeOf part while the code works even without: https://stackoverflow.com/a/74089583/1009466 – Toniq Oct 20 '22 at 17:06
  • @Toniq ... **1)** The system you came up with **a)** forces everyone into writing the dispatching part in a non standard way which despite the method's name (`dispatchEvent`) allows passing multiple arguments **b)** does not check the reliability of the dispatching source which invites developers of messing around with it even in case one would agree to just dispatching a single event like object at time. **2)** Regarding your above mentioning of `setPrototypeOf `, adsy's example code tries to solve your problem from the inheritance perspective and not form the mixin-based composition side. – Peter Seliger Oct 21 '22 at 13:16
  • @PeterSeliger - 1) I understand, thanks for the info, but this is enough for me. 2) his code also has this part: EventDispatcher.call(this); which I used, but not setPrototypeOf , thats why I was wondering do I need this? I dont want my code failing at some point. – Toniq Oct 21 '22 at 13:37
  • @Toniq ... your current fiddle code does it all. `EventDispatcher` gets used as function based mixin. No inheritance, thus, no prototypes needed. – Peter Seliger Oct 21 '22 at 13:59
  • @Toniq Try defining the methods as `EventDispatcher.fireEvent` etc and you'll notice the difference – Bergi Oct 21 '22 at 14:17

1 Answers1

1

When you do PlaylistManager.prototype = new EventDispatcher();, every instance of PlaylistManager will be now be attached to one single event dispatcher. You have created one EventDispatcher and assigned it as the canonical one for all PlaylistManager's. That means you'll get all instances sharing the same events.

You most likely want one owned per PlaylistManager. See the MDN docs.

(function(window) {
  "use strict"
  var PlaylistManager = function(data) {

    var self = this

    EventDispatcher.call(this);

    this.test = function() {
      self.fireEvent('PlaylistManager.COUNTER_READY', [{
        counter: "a"
      }]);
    }

  }

  Object.setPrototypeOf(
    PlaylistManager.prototype,
    EventDispatcher.prototype,
  );

  window.PlaylistManager = PlaylistManager;

}(window));

Side point, but in my opinion it's easier to reason about ES6 classes rather than use the prototypical inheritance model directly.

adsy
  • 8,531
  • 2
  • 20
  • 31
  • You still did forget the `super` call that would create an individual `.events` on every `PlaylistManager` – Bergi Oct 16 '22 at 19:42
  • What would that look like here? Admittedly I'm rusty on using prototypical inheritance like this because as the commenters on OP mention, there's generally better patterns. – adsy Oct 16 '22 at 20:54
  • Ok ive added `EventDispatcher.call(this);` which i think will do the trick? – adsy Oct 16 '22 at 21:23
  • Yes, that's what I meant, thanks! And no, the prototypical inheritance pattern is totally fine, a mixin is just different not "better". (Though it would work better if the methods were actually defined on the prototype, of course). – Bergi Oct 16 '22 at 21:25
  • Sorry didn't mean to impose -- its up to you and your project what patterns you use! This is a limited example and fine. – adsy Oct 16 '22 at 21:34
  • @adsy - it this correct? https://jsfiddle.net/ng50cw6b/2/ It works even without adding this prototype code. – Toniq Oct 19 '22 at 07:36