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 MediatorMap [OLD] #30

Closed darscan closed 12 years ago

darscan commented 13 years ago

An issue to discuss the Mediator Map API and implementation.

Stray commented 13 years ago

I've restarted this thread here with less confusion: https://github.com/robotlegs/robotlegs-framework/issues/46


In RL1 we detect the extension of UIComponent by the view, and if we find it then we wait for creationComplete.

I'm wondering whether we could do away with all that hidden checking, and instead have a function that is used in the mediator's constructor if required:

waitForEvent(eventType:String, eventClass:Class);

This will also let people customise which event should be waited for before onRegister is run, and I think it'll help people to better understand the difference between preRegister and onRegister.

Alternatively, we could have DisplayObjectMediator and UIComponentMediator. The reason I'm not a big fan of the subclassing is that when they're further subclassed to add - for example - sugar for adding signals, it gets annoying - though we might be able to elegantly solve that with package level functions in the extensions.

Thoughts?

darscan commented 13 years ago

[I see that a new comment has just come in.. here are my thoughts anyway]

So here's where my current thinking is (may be complete rubbish as is often the case):

One of the (many) problems with the RL1 mediator implementation was that the class was doing too much. The mediator that the user writes should do nothing other than mediation. It should be given a view, and it should start and stop mediating.

It shouldn't be responsible for dealing with the life-cycle methods of the underlying ui framework - be that Flex, MinimalComps, Starling, or some other ui component framework. It shouldn't be concerned with re-parenting.

And the mediator map shouldn't be concerned with those things either.

If I change the underlying ui framework, but don't change the view API as exposed to the mediator, I shouldn't have to edit the mediator (to change its superclass for example). It is still mediating the same view api. What I should be able to do is tell the mediatorMap to use a different mediator management strategy (per mapping or for the whole map). The strategy would be designed to handle the idiosyncrasies of the view component's underlying ui architecture (like a custom flash component.. think timeline where the view isn't ready until some transition has completed).

darscan commented 13 years ago

exactly, but it may not always be an event dispatched by the view that we need to wait for. It may indeed be some other peculiarity.

darscan commented 13 years ago

boo to subclassing! ;)

darscan commented 13 years ago

preRegister() and onRegister() were hideous mistakes!

jeremyruppel commented 13 years ago

This is definitely one issue that hit close to home for me. I could never find a good of where to put, for lack of a better term, the "view logic" for a given view. It doesn't belong in the mediator because the mediator's job is communication between the view and the context, but in many cases putting it back in the view felt more like heavy view components which is one of the problems we're trying to solve.

I'm not 100% convinced that this is a good suggestion but I'm going to throw it out there anyway: we could go with a Mediator and and ViewController for a given view component. The Mediator could still be responsible for communication to and from the ViewController, the ViewController could conform to a specific interface that tells us about the view lifecycle (when it's ready to be mediated, etc). This also gives us a decent place to put the "view logic" if there happens to be any.

Stray commented 13 years ago

You know I always enjoy the strategy pattern...!

I did think about whether the 'not an event' thing is relevant, and yes - could be a signal - but if the view is doing its own thing then I can't think of a sensible approach other than to have it notify the outside world.

I don't think I'd want the mediator poking into the view ... so it seems like messaging (by default via events) is a sane assumption. So, I'm thinking that waitForEvent() covers it.

Also - it's quite easy to implement your own Mediators now, so for the 20% that this doesn't cover, folk can bend their own solution.

Agree that preRegister and onRegister are horrible... the DuckTypedMediatorTrigger should allow us to change the API without stopping it from being possible to use old styleeee mediators.

Stray commented 13 years ago

On API - I quite like "startup" and "shutdown" for the external funcs, and internally overriding something protected and explicit like "wireEvents". Might help people to avoid doing their random jobs in onRegister too :)

Stray commented 13 years ago

@Jeremy - check out Hooks - they allow you to very easily wire your viewController if needed, including making it stateful (more details on that a little further down the line).

darscan commented 13 years ago

Hmm.. I think perhaps I'm just really bad at explaining this stuff. I'll take a stab at the adapter/strategy/trigger approach and we can compare notes :)

Stray commented 13 years ago

I'm just saying that almost all views are perfectly suited to just picking up an event when you get going. And the easiest place to configure that per mediator is in the mediator.

It means that when you're wondering "Why isn't my onRegister running?" and looking at the trace that never comes, you're simultaneously looking at the waitForEvent(SomeEvent.SOMETHING_THAT_NEVER_HAPPENS) and have a chance of figuring it out!

This would definitely cover a lot more than the 80%, and with a tiny barrier to entry...

Stray commented 13 years ago

But yes - please do at least sketch what you're intending - I think words haven't really exposed it to me. Code or just a diagram would be wicked.

I was thinking that because you're really only hoping to run a function at the end, doing something other than waiting for an event is only as simple as putting a handler in the mediator that uses onRegister (or whatever it's called) as a callback - which you could abstract to an object or a package level function if you were reusing it a lot.

So - we're not cutting off other possibilities just by making it expect an event (or nothing) out of the box.

darscan commented 13 years ago

Coolio, do what feels right for now - we will see how it feels and review it all later anyway. Options FTW!

Stray commented 13 years ago

Yup - and at least the tests will cover the 'custom event' approach, so refactoring to a more explicit strategy pattern should be feasible against the tests without redoing them.

joelstransky commented 13 years ago

Just my humble two cents, I'm semi on board with the constructor parameter but not if it requires me to dispatch a custom event from my view. A big part of my love for RL comes from not having to consider the framework while writing my views. What is the fundamental problem with onRegister?

Stray commented 13 years ago

Hi Joel, the default would be that it doesn't expect any event, so it doesn't change that side of things.

However, Shaun is right, there's something I missed (you also need to check whether the flex view is already initialised) so we need a more flexible approach.

joelstransky commented 13 years ago

I see, I suppose a trigger is needed in some fashion. I was never really a fan of ADDED_TO_STAGE as said trigger. The ability to append custom strategies to accomodate minimalcomps for example might be nice for the 20%

@Jeremyruppel, dunno if this is the correct thread to discuss the role of mediators so sorry if this is off topic but I have the same conflict. I have a year+ long RL project that does a considerable amount of business logic in some mediators. I just couldn't bring myself to make my models and services dependencies of my views so my mediator handlers evaluate them instead and ultimately call view api rather than simply inform them that something happened.

neilmanuell commented 13 years ago

Having some sort of Trigger would keep it in line with the new CommandMap impl. The map, (here the MediatorMap) should just maintain the mappings, some other class does the event listening. If this separation occurs in one, but not another it might cause confusion??

BTW, (without any judgement intended), the waitForEvent sounds a bit similar the pMVCs handleNotificationInterests, but with a totally different purpose. Actually, maybe there was a judgement in there and I was just being nice (but dishonest). Why should the Mediator know when it is to be registered? (unless I have misunderstood the purpose of the waitForEvent param)

Stray commented 13 years ago

Hi Joel / Neil - the ADDED_TO_STAGE is still the trigger for a mediator being created. That's not the same as the trigger we're discussing here - which is about when it's safe to run onRegister.

And also the ADDED_TO_STAGE stuff is handled somewhere else entirely - the mediatorMap in V2 simply receives notifications. And then (just to fill you in) if it didn't care to manage that particular object then it never receives another notification for any object of that class... how efficient is that?! (Obviously we clear this when a new mapping is made as it might have a new interest).

There already is a trigger for mediators which then does the setViewComponent / preRegister / preRemove work. This trigger is 'loaded' to the mediatorMap and allows you to specify v1 Mediators, v2 Mediators or Duck Typed Mediators, with the option to be 'strict' - which will throw an error if a mediator is created which doesn't fit expectations - or not strict - which just ignores mediators that don't fit the expectation, allowing you to use [PostConstruct] to entirely role your own mediator which doesn't have any relation to what Robotlegs prescribes.

Phew!

So - we're not really talking about the same things here. (Sorry for the confusion).

Having tinkered with it more, waitForEvent doesn't work - well, it only works first time around, after that it's no good, so we need to ignore that suggestion that I made - it was a false path.

The only reason for having the mediator know when it's safe is that mediators vary for views and if you've got a Flex UIComponent then you need to wait for creationComplete (first time around), and if you've got a pure AS3 sprite then you don't.

So - to clarify, this has nothing to do with the creation of mediators, but to do with managing the gap between the preRegister and onRegister functions running.

But, I'm increasingly thinking that I need to return to the existing mediator functionality where it's hidden from the user, because co-variant mediation means that if two views - one UIComponent and one Sprite - both use the same mediator then we can't use the strategy I was thinking of.

In the current RL1 mediator implementation, the mediator hides this from the user, but it's still going on under the hood. I think I'm going to return to that, as it certainly worked for nearly everybody, nearly all the time, but I'll pull out the actual code that checks for waiting into its own function, to make it simpler to override and put in your own custom behaviour. Having gone around the houses I can actually see why it was done the way it was originally!

neilmanuell commented 13 years ago

Ah, i see... in that case I would say waitForEvent certainly is more confusing than the RL1 way. though academic now I guess.

Stray commented 13 years ago

Yup! Sorry for all the confusion - I've got a new idea and I'm up and running - will shout as soon as there's code to look at and you can all chip in on something that makes more sense!

Stray commented 13 years ago

I've restarted this thread with less confusion here:

https://github.com/robotlegs/robotlegs-framework/issues/46

tschneidereit commented 12 years ago

Duplicate of #46