marcojakob / dart-event-bus

An Event Bus using Dart Streams for decoupling applications
MIT License
755 stars 82 forks source link

Because event_bus uses mirrors it's less usable for client side #12

Closed zoechi closed 9 years ago

zoechi commented 9 years ago

This is a simple but very useful event bus implementation but the use of mirrors is very inconvenient for client side use. Would be great to have the old implementation back (and maintained) maybe as an alternative in it's separate library.

I heard complaining others about this and just want to make you aware of the problem. In my projects, I currently use a fork anyway and am not affected currently.

marcojakob commented 9 years ago

Thanks @zoechi for pointing that out. It is really hard for me to tell how many people are happy or unhappy users of event_bus. I guess since it is such a small and simple library, people unhappy with it just create a fork and use that.

I would love to have a discussion of how improvements could be added back to this library.

Use of Mirrors

Mirrors are only used for the HierarchicalEventBus which allows listening to events of a specific class and its subclasses. The normal EventBus only uses the runtimeType to filter events.

I don't know if people are actually using the HierarchicalEventBus (please speak up).

We could also put the HierarchicalEventBus into a separate library file. That way the normal EventBus would not depend on mirrors and people could opt in for the hierarchical bus that depends on mirrors.

What do you think?

zoechi commented 9 years ago

This is why I decided to give you a note :-)

Having two versions 'normal EventBus' without mirrors and 'HierarchicalEventBus' in two separate libraries seems perfect to me.

marcojakob commented 9 years ago

Would be great to have the old implementation back

I still think that the current implementation is better and much simpler than the old. Here is a comparison:

Old Version (v0.2.4)

// Define Event
final EventType<User> userLoggedInEvent = new EventType<User>();

// Register Listeners
eventBus.on(userLoggedInEvent).listen((User user) {
  print(user.name); 
});

// Fire Events
User myUser = new User('Mickey');
eventBus.fire(userLoggedInEvent, myUser);

New Version (v0.3.0)

// Define Event --> Is not needed because any Dart class can be used as an event.

// Register Listeners
eventBus.on(UserLoggedInEvent).listen((UserLoggedInEvent event) {
  print(event.user);
});

// Fire Events
User myUser = new User('Mickey');
eventBus.fire(new UserLoggedInEvent(myUser));

Do you agree or do I miss something here?

zoechi commented 9 years ago

I think it's fine. If anybody wants additional features she should just create a feature request/bug report ;-)

marcojakob commented 9 years ago

Fixed in v0.4.0 (see 6a3c6cc539539c8b610b8e91371e610e7fd97de8).

Moved the HierarchicalEventBus to a separate library to be able to remove dart:mirrors from the normal EventBus.

Users of the hierarchical event bus must import event_bus_hierarchical.dart and replace the use of the factory constructor EventBus.hierarchical() with the HierarchicalEventBus constructor.