Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added an event dispatcher filtering mechanism #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

miraclebg
Copy link

  • Implemented event dispatcher filtering (aka pass a variable so all listeners can alter it and return it back)
  • Code formatting

@krasimir
Copy link
Owner

krasimir commented Jul 3, 2016

Hey,

Thanks for the PR. A few things:

  • The suggested method looks more like reduce rather then filter. My feeling is thatfilteraccepts a list of items and returns another list of items whilereduce` accumulates a single value.
  • Such a method is a little bit out of the scope of the library. For me it sounds like the event listeners need to do an additional work over a variable and because they are part of the EventBus we are adding a new method to perform that work. I'll rather go with the following:
EventBus.addEventListener('my-event', callback1);
EventBus.addEventListener('my-event', callback2);
EventBus.addEventListener('my-event', callback3);
EventBus.addEventListener('my-event', callback4);

var listeners = EventBus.getListeners('my-event');

value = { ... };
var result = listeners.reduce(function (r, listener) {
  // some logic here
  return r;
}, value);

The missing part is getListeners method. So, instead of creating a filter method I'll probably create getListeners that returns all my functions in an array and then use reduce on top of that.

  • +1 for the code formatting.
  • The new method is not documented in the README

What you think if you add getListeners method and try using it? If it works I'll be happy to merge this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants