search

GeppettoJS / backbone.geppetto

Bring your Backbone applications to life with an event-driven Command framework.
http://geppettojs.github.com/backbone.geppetto/
MIT License
203 stars 28 forks source link

allows batch registration of several commands for a single event #20

Closed creynders closed 11 years ago

creynders commented 11 years ago

Adds the possibility to register several commands to the same event in batch:

commands: {
    "appEventFooBarBaz":    [
        FooCommand,
        BarCommand,
        BazCommand
    ]

Sequentially handled.

Didn't bump the version number, nor build the .min file. And sorry 'bout the EOL shizzle. You can view the proposed changes without whitespace diffs by appending &w=1 to the URL.

creynders commented 11 years ago

I understand if you hesitate with this one. I'm not fond of inner loops either, but after thinking it through I thought it'd be ok, since this happens at mapping time and will be invoked with small arrays generally.

Another option would be to add a straight Context#execute method, sth like:

context.execute( FooCommand, BarCommand, BazCommand);

I think I'll be creating a PR for that one as well, but first I'd like to sort out the entire event payload stuff as partly mentioned in #18, since it'll influence a potential Context#execute method as well. Also, ATM there's a ... um ... discrepancy or asymmetry between how events+payloads are injected into commands and how they're passed to event handlers : a command is injected with an eventData and eventName, while an event handler receives a single argument where eventName is injected into an eventData-like object.

Just throwing ideas and remarks around here, you definitely don't have to agree with them. And feel free to reject my PR's, they're just stuff that make sense to me, but aren't necessarily going in the direction you'd like to go.

geekdave commented 11 years ago

Merged in commit https://github.com/ModelN/backbone.geppetto/commit/cd25126e19d66c2092dda5e1c98ff33d40cae2d3

I had to do it manually due to some merge conflicts (perhaps due to the whitespace stuff).

Definitely no hesitation at all... I think it's a great feature!

Yes, the eventName/eventData discrepancy is ugly. I don't like the idea of injecting another named parameter into someone else's event payload. Perhaps it would be cleaner to just pass the event name as the 2nd parameter. This was really only done to facilitate "re-dispatching" of events in application code.

Thanks for the contributions! I tagged a new release and gave you a shout-out in the readme. :) Please keep them coming.

creynders commented 11 years ago

Thanks for the shout-out :)