robotlegs / robotlegs-framework

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

Make Modular Event Relaying Easier #118

Closed darscan closed 11 years ago

darscan commented 11 years ago

http://knowledge.robotlegs.org/discussions/robotlegs-2/931-intermodular-communication-bestpractice

creynders commented 11 years ago

I've started on this one, BTW.

creynders commented 11 years ago

So, the API I'd propose is something like this:

//IModuleConnector extens IModuleConnectionAction
function onChannel(channelId:String):IModuleConnectionAction;
//IModuleConnectionAction
function relayEvent(eventType:String, eventClass:Class = null):IModuleConnectionAction;
function receiveEvent(eventType:String, eventClass:Class = null):IModuleConnectionAction;
//global sending
moduleConnector.relayEvent(FooEvent.FOO, FooEvent);
eventDispatcher.dispatchEvent(new FooEvent(FooEvent.FOO));

//global listening
moduleConnector.receiveEvent(FooEvent.FOO, FooEvent);
eventCommandMap.map(FooEvent.FOO, FooEvent)
    .toCommand(FooCommand);
//explicit module-to-module sending
moduleConnector.onChannel('a-b')
    .relayEvent(FooEvent.FOO, FooEvent);
eventDispatcher.dispatchEvent(new FooEvent(FooEvent.FOO));

//explicit module-to-module listening
moduleConnector.onChannel('a-b')
    .receiveEvent(FooEvent.FOO, FooEvent);
eventCommandMap.map(FooEvent.FOO, FooEvent)
    .toCommand(FooCommand);

Basically it would be a eventdispatcher map and would use named IEventDispatcher mappings behind the scenes.

This also got me thinking that this would be the 3rd concept to use the whole map-as-conrete-event or map-as-abstract-event shebang (EventCommandMap and EventMap being the other two) Maybe it'd be a good idea to create a StrictEventDispatcher which takes care of all that shizzle and which is used by the other 3 concepts? I haven't really looked into it yet what it would take though.

darscan commented 11 years ago

Maybe it'd be a good idea to create a StrictEventDispatcher

Hmm.. I hear you, but I'm not sure. With the simple Event Relay I chose to avoid doing any Type checking. The events are relayed regardless of type, and it's up to the consumers (EventCommandMap for example) to do the Type checking.

creynders commented 11 years ago

@darscan where exactly is the connection between a shell's context/injector and a module's context/injector made? I've been searching around, but can't seem to find anywhere how the module injector receives a reference to the shell injector. TBH I don't know where to look either.

darscan commented 11 years ago

Check out the Modularity Extension. It happens in one of the two *ExistenceWatchers.

creynders commented 11 years ago

I've been working on this in 15 min sprints, think I can wrap it up tomorrow morning. Good enough to get you to review it anyway. Think you'll want to rename stuff, the impl itself is pretty simple though.

darscan commented 11 years ago

Rad. Perhaps the EventRelay could be used here (maybe with some modifications):

https://github.com/robotlegs/robotlegs-framework/blob/master/src/robotlegs/bender/extensions/eventDispatcher/impl/EventRelay.as

creynders commented 11 years ago

https://github.com/creynders/robotlegs-framework/tree/modularity

darscan commented 11 years ago

Awesome! Looks really good. I like the ModuleConnectionConfiguratorTest style (the mocks).

darscan commented 11 years ago

With this in place do we still need the ScopedEventDispatcherExtension?

darscan commented 11 years ago

Also, we might need to be careful with the word "global". As far as I can tell, there is no "global" dispatcher, just "top-level" ones.. the whole parent-child thing.

darscan commented 11 years ago

Anyhoo, I'm going to merge this into master. We can review from there.

darscan commented 11 years ago

Merged (and slightly tweaked). I haven't delved in fully, but from a glance it looks great.

creynders commented 11 years ago

With this in place do we still need the ScopedEventDispatcherExtension?

Nope, I don't think so!

As far as I can tell, there is no "global" dispatcher, just "top-level" ones.. the whole parent-child thing.

Maybe it should be a bit more obscure, so people won't accidentally be messing around with it.

I haven't delved in fully, but from a glance it looks great

Super.

darscan commented 11 years ago

Maybe it should be a bit more obscure, so people won't accidentally be messing around with it.

What should we call it? Something to do with shell? parent, topLevel, super, outer, root

creynders commented 11 years ago

Thing is... none of those are really correct either. Basically it's a default named IEventDispatcher that's accessible through all injectors, but so are all others created with the ModuleConnector, except you choose the name yourself. Maybe something like defaultModuleChannel ..?

darscan commented 11 years ago

Basically it's a default named IEventDispatcher that's accessible through all injectors

Hmm.. even that's not quite correct (AFAIK). It's only accessible to a given context hierarchy.

darscan commented 11 years ago

I removed the getRootInjector() package-level function - it only had a single callsite, so I inlined it.

creynders commented 11 years ago

I removed the getRootInjector() package-level function - it only had a single callsite, so I inlined it.

Ah yes. Good habits die hard ;)

darscan commented 11 years ago

So a big difference between this (using getRootInjector) and the ScopedEventDispatcher is that this one always creeps up to the top level of a context hierarchy. In other words a module can't create a bus that is only available to itself and its children - instead the bus will be available to every context in the tree.

In reality I don't think that's really a problem.

darscan commented 11 years ago

Regarding IModuleConnector.globally(), perhaps is should be onDefaultChannel.. although that is rather long.

darscan commented 11 years ago

onSharedChannel(), onShared()

creynders commented 11 years ago

So a big difference between this (using getRootInjector) and the ScopedEventDispatcher is that this one always creeps up to the top level of a context hierarchy.

Yeps.

In reality I don't think that's really a problem.

Yeah, I think so too.

perhaps is should be onDefaultChannel.. although that is rather long.

I like that one best though...

darscan commented 11 years ago

I renamed it to onDefaultChannel. I also removed the ScopedEventDispatcherExtension.