robotlegs / robotlegs-framework

An ActionScript 3 application framework for Flash and Flex
https://robotlegs.tenderapp.com/
MIT License
967 stars 261 forks source link

RL2 CommandMap #31

Closed darscan closed 12 years ago

darscan commented 13 years ago

An issue to discuss the Command Map API and implementation.

pixels4nickels commented 13 years ago

Are there any plans to roll in a signalCommandMap like Joel wrote? I was peeping the latest RL2 codebase and saw that the command map is more easily extendable with the new API work. Am I missing something that is already in there that will allow me to map signals for command execution?

darscan commented 13 years ago

Yup, commands are now kicked off by command triggers. The command map itself doesn't need to know anything about events or signals. Instead it is up to the triggers to deal with attaching listeners, passing parameters and executing the commands. It's still early days for that though.

pixels4nickels commented 13 years ago

That is more than I could ask for. No further questions for the witness!

honi commented 13 years ago

I'm interested in having a CommandMap trigger that works with SWFAddress, for mapping external url changes to commands. Going to look into that!

neilmanuell commented 12 years ago

What are the use-cases for adding hooks to a mapped command?

darscan commented 12 years ago

A hook has the opportunity to manipulate a command instance before execute() is called. I know, not an actual use case, but guards and hooks are designed to DRY up commands and mediators.

darscan commented 12 years ago

At the moment the command map (and mediator map) API is:

map(Response).to(Request);

Whereas, the injector API is:

map(Request).to(Response);

The latter feels more natural. This concerns me a fair deal :)

tschneidereit commented 12 years ago

I agree: Request -> Response is a far more natural mapping.

Maybe we could change the API along the lines of

mapEvent(FooEvent.BAR).to(BarCommandClass);
mapTrigger(fooTrigger).to(BarCommandClass);

?

neilmanuell commented 12 years ago

@darscan lol well I figured that is what hooks allowed, but couldn't think of a reason for doing this.

darscan commented 12 years ago

@neilmanuell Well, we can always pull hooks out of the command map later if we can't come up with, you know, an actual valid use case :)

pixels4nickels commented 12 years ago

I would love to see an interceptor pattern implemented. I have a lot of use for them and got "hooked" :P on them in PureMVC Fabrication. Are the guards going to supply this functionality? Will they be able to "intercept" a command before it hits the mediator or command(both)? That would rock.

pixels4nickels commented 12 years ago

If you are not familiar with Fabrication Interceptors here is a breakdown: http://code.google.com/p/fabrication/wiki/Interceptors

darscan commented 12 years ago

Yup, guards and hooks provide pretty much the same functionality. With the exception of passing arbitrary parameters. I've opened an issue here: https://github.com/robotlegs/robotlegs-framework/issues/53

darscan commented 12 years ago

The current style - map(BarCommand).to(FooEvent.BAR).withGuards(HappyGuard) - puts emphasis on the command. We're saying: "I have this command in my system, with a set of guards and hooks, that can be triggered in multiple ways".

commandMap.map(BarCommand)
  .withGuards(HappyGuard)
  .toEvent(FooEvent.BAR)
  .toEvent(FooEvent.BAR2)
  .withHooks(SomeHook);

The command is the subject of the mapping, and there can be only one mapping per command. So, assuming the code above, we could later open up the mapping and add more triggers and hooks:

commandMap.map(BarCommand)
  .withGuards(GrumpyGuard)
  .toEvent(FooEvent.BAR3);

This would (perhaps deceptively) add the new guard and trigger to the old mapping. There is no way to execute that command with a different set of guards or hooks. All triggers for that command use the same mapping.

The proposed style - mapEvent(FooEvent.BAR).toCommand(BarCommand).withGuards(HappyGuard) - puts emphasis on the trigger. We're saying: "I have this event/signal/whatever in my system that can trigger multiple commands with this set of hooks and guards".

commandMap.mapEvent(FooEvent.BAR)
  .toCommand(BarCommand)
  .withGuards(HappyGuard)
  .withHooks(SomeHook);

Here the mapping would be per trigger. The same command could be executed by different triggers, each with its own set of guards and hooks. Also, that trigger could be wired to multiple commands:

commandMap.mapEvent(FooEvent.BAR)
  .toCommand(BarCommand)
  .toCommand(BarCommand2)
  .withGuards(HappyGuard)
  .withHooks(SomeHook);

But now you can only have one mapping per event. The following would open up the old mapping and add new commands to it:

commandMap.mapEvent(FooEvent.BAR)
  .toCommand(BarCommand3);

Also, you can't just unwire a command - you have to know how it was being triggered.

I see pros and cons to both approaches.

Do we want:

A. A command to have its own configuration (guards and hooks) and multiple mechanisms for firing it off, or B. A trigger to be able to kick off multiple commands using the same set of guards and hooks?

Thoughts?

Stray commented 12 years ago

Good Questions!

The way the mediatorMap mappings work, a mediator can be mapped to many views, and the guards and hooks belong to the view-mediator mapping, so each mediator-view (or matcher) pairing has unique guards and hooks.

So - I want to have my cake and eat it. I think the guards/hooks apply per command/trigger pairing, not solely to a trigger or solely to a command.

Stray commented 12 years ago

My use case for that is that I may have the same command wired to 3 different events, and the conditions for the guarding on the command could be unique to each event (ie reads some property of the event, or has to check a different model). For example - it might be a 'PreventQuitCommand' and the event/condition which triggers that may vary a great deal.

darscan commented 12 years ago

Ok, I've flipped it. It turned out to be the right way to go IMO. There is now one mapping per trigger-command combo.

I've also stripped all the event stuff out of the command map. There is now a separate EventCommandMap extension. The EventCommandMap does not extend CommandMap - it composes it and provides an event specific API that delegates to the command map.

neilmanuell commented 12 years ago

@darscan just to let you know that I've been experimentally re-purposing the CommandMap, and found that your separation from Observer implementation from the Map makes it far easier to adapt and test

thanks

darscan commented 12 years ago

@neilmanuell cool, good to hear. Have you played with the latest command map api/impl?

pixels4nickels commented 12 years ago

@stray : That is a heavy use case for me as well. I need to be able to use different guards/hooks for different implementations of the same command. Passing arbitrary params would be the icing on the cake.

neilmanuell commented 12 years ago

@darscan Only by pulling it apart to see how it works, then putting it back together as a ProcessMap (I'll share eventually). I did it for the older version, then just as I had finished the tests, you updated. So I re did everything with the newer version and felt it came together a lot cleaner, faster, and far easier to test. I haven't played around with it as a CommandMap yet, but that will come.

Stray commented 12 years ago

@actionscriptguy

On arbitrary params - I'm leaning towards saying "if it's state, stick it in a model" - to me, putting state in the wiring feels a bit messy, but we can look again at that when the core is built.

We've decided on the guards-hooks being attached to the wiring, not either the Command or the Trigger, so that's good :)

darscan commented 12 years ago

@neilmanuell awesome, all good signs!

pixels4nickels commented 12 years ago

Sounds great! I agree the case for arbitrary params can be easily (and more properly) solved by keeping those params on a model. This jives especially well with my signals/presentation model style implementations.

pixels4nickels commented 12 years ago

I got lost but found my way back.

darscan commented 12 years ago

Ok, the CommandMap and EventCommandMap feel pretty resolved now. Closing, feel free to re-open.