Home → Questions →

Best practice... want a reusable parent view mediator to be aware of things happening in a resuable child view?

rickcr

22 Mar, 2010 07:16 PM via web

Admin Functions

• Edit
• Remove

I want to be sure I’m doing things the RL way, but I’m running to a common issue quite frequently that I could use some help on...

To start with let’s assume the following

ViewA which has ViewA_Mediator

ViewA has a ViewAsubComponent on it and thus there is also ViewAsubComponentMediator

In other parts of the app, ViewA, ViewASubComponent also can be reused.

When the user clicks an ‘ok’ button on ViewASubComponent, I want to scoop up some things done on ViewASubcomponent and ultimately make the ViewA Mediator instance (ViewA_Mediator) aware of what happened within the child view.

The problem is if I keep things loosely coupled, I seem to have to make a system notification event from ViewASubcomponentMediator that its (somewhat) parent ViewA_Mediator can listen for. This gets dicey when I’m reusing the components in different parts of the app since all the instances would be notified of the event and I’d have to have logic in the handler to make sure only the correct instance is being processed. This makes me think I'm missing something fundamental that I could do to avoid this issue.

All this goes away if I sort of ‘cheat?’ and attach my listener to the subViewAComponent directly from the parent ViewA_Mediator
eg:

ViewA_Mediator onRegister()...

viewA_layout.theSubViewAcomponent.subViewButton.addEventListener(MouseEvent.CLICK, onOk)

Is the above an ok ‘cheat?’ or is there a cleaner way to handle what I’m trying to accomplish? - which is basically a clean way to have a parent view be notified of things happening in its child view? Maybe I'm missing something but it seems like a parent view will know about child views it creates, but the backing mediators really don't know about each other. It's as if I actually want some tight coupling in this case.

Showing the most recent page of comments. View the first page

1. Support Staff 32 Posted by Stray on 08 Feb, 2011 03:47 PM

   

   Hi Glen,

   Agreed - it would definitely be possible to do that, but IMO it makes integration testing harder, and I think if we accept that Signals are a base type, we don't need to further guard against their use.

   You'd also need to add the de-register for notifications side as well. And it would make the automatic clean-up by a signal mediator much harder if not impossible - as that effectively relies on the 'sameness' of every signal in order to provide a 'SignalMap' that unmaps automagically when the mediator leaves the stage.

   It's interesting though - where is the line? I'm a real stickler for doing things 'right' - but with Signals I feel like it's 'right enough' to use them through the interface directly. Wonder why that is? I don't think it's just laziness, as I do favour a lot of other 'more work' solutions.

   Perhaps I feel like signals are a kind of loose coupled glue in themselves? They allow relationships-by-composition instead of inheritance - and I like that? I like that they make it possible for a SignalMap to manage the relationship.

   Be interested to know what Robert Penner, Joel and co think about this too.

   Good point though.

   Admin Functions

   • Edit
   • Remove

2. Support Staff 33 Posted by Till Schneidereit on 08 Feb, 2011 03:57 PM

   

   I agree with Stray: Signals should be viewed as a part of the view's
   API, where the fact that you have to use two dots to interact with
   them is a problem with the language's syntax and semantics, not with
   what the view's author did.

   I oh-so-much which we had either C#'s Delegates, which Signals is
   based on, but which are properly included into the language itself,
   or, perhaps even better, Traits as in Scala. With the former, there'd
   be no need for Signals, great as they are, while with the latter,
   integrating them, and much else, into the API of a class would be a
   non-issue.

   But as long as we don't have them, I think we should accept the fact
   that not every piece of code is going to look as nice and tidy as it
   should. At least keep the ugly in corners you know your way around in.

   But now I start thinking about what pattern matching or a better type
   system would allow me to do and get sad ...

   Admin Functions

   • Edit
   • Remove

3. 34 Posted by Glen Whitbeck on 08 Feb, 2011 04:46 PM

   

   Hi Stray,

   I'm curious why you feel it would make testing harder. The only thing that really changed is we decoupled the mediator from the signal. My initial instinct is to think that loosely-coupled objects are easier to test than more tightly-coupled objects. This method would give us a point to override/mock. Without it, we have to deal with the fact that the mediator automatically uses this other object (signal).

   As far as cleaning-up, I'm trying to think through the possible complications. But, my initial thought is that we haven't really changed anything. In both cases, we are handing a mediator function to the signal. And, it both cases, the signal "belongs" to the view. I didn't understand the part about the "sameness" of the signal … we are using the same signal as before. Maybe I should dig-through the clean-up logic to gain some more insight.

   I definitely agree that it can be difficult to decide where to draw the line. And, I'm not sure this level of decoupling is necessary (it was just a thought for @djaffy … something that might make things more conceptually comfortable). However, it might make the code more readable. The method name might make it immediately obvious what is happening … as opposed to stepping through the dots and thinking about the method names meaning on other objects. But, that's probably in the eye of the beholder … and pretty marginal, in this case.

   Admin Functions

   • Edit
   • Split
   • Remove

4. 35 Posted by Glen Whitbeck on 08 Feb, 2011 04:57 PM

   

   Hi Till,

   I think I can go along with that. It's probably reasonable to make an exception for signals on the grounds that that functionality should really be part of the language itself.

   It's funny, though … it makes me argue with myself! I ask myself, "Why do we make an exception for this object? What if it was some other object type? If it was an object called ReallyDangerousObject with a method called showMeTheSecretInnerDetails() … would we still be ok with that? It seems like we're either talking to our friends' friends or we're not."

   But, I think I'll respond to myself with your reply … perhaps this situation is uniquely different because this functionality should be part of the language and not a responsibility of the view.

   Admin Functions

   • Edit
   • Split
   • Remove

5. Support Staff 36 Posted by Stray on 08 Feb, 2011 05:14 PM

   

   Hi Glen,

   it's probably easiest to look at the code for the SignalMediator mappings. Basically, in order to clean up when the mediator is automatically removed, instead of adding to signals directly, we use a utility function:

   addToSignal(view.someSignal, someHandler);
   

   This just runs the following:

   protected function addToSignal(signal:ISignal, handler:Function):void
   {
       signal.add(handler);
       if(handlersBySignal[signal] == null)
       {
           handlersBySignal[signal] = [];
       }
       handlersBySignal[signal].push(handler);
   }
   

   Now the SignalMediator keeps a record of the added signals (I should really break this out to a SignalMap and do it by composition, but that's not relevant to this example).

   When the mediator is removed, in preRemove, the SignalMediator does:

   removeAllSignalHandlers();
   

   which runs the following:

   protected function removeAllSignalHandlers():void
   {
       for( var signal:Object in handlersBySignal)
       {
           var handlers:Array = handlersBySignal[signal];
           var knownSignal:Signal = ISignal(signal);
           for each(var handler:Function in handlers)
           {
               knownSignal.remove(handler);
           }
       }
   
       _handlersBySignal = null;
   }
   

   Consider how you could implement this if you wrapped each signal addition in an API function on the view.

   Somehow, the SignalMediator (or SignalMap) would need to know that if the mediator did:

   view.registerForNotificationsForClick(clickHandler);
   

   the corresponding removal function is

   view.deregisterForNotificationsForClick(clickHandler);
   

   Hmm... so - we could, I guess, always return a de-registration function from the registration function, but we're into really, really dodgy ground in terms of compiler checking now, because where ISignal.remove(handler) is a known API, if we return a function from the registration method we have no way of knowing what the signature of that function is.

   The beauty of Signals is that every Signal employs the same ISignal interface. So you can couple your code to that generic interface, and not to the view API - for the purposes of the notification architecture. The view API hands you a more sophisticated contract to work with.

   The point here is that Signals really should be a first-class citizen. When you integrate the Signals library you can make the decision that every relevant Class in your code can have knowledge of the ISignal contract without that breaking encapsulation - in the same way that it feels ok for any Class to use Math or String or Array.

   I think the Law of Demeter should really be called the "Mostly Useful Guideline of Demeter".

   Does that make sense? Or am I missing something clever that would allow SignalMap/Mediator to work and have compile time checking without having to expose the signal itself?

   Stray

   Admin Functions

   • Edit
   • Remove

6. Support Staff 37 Posted by Stray on 08 Feb, 2011 05:17 PM

   

   Glen - I missed the bit about the testing.

   If the Signal is a property on the API, your test code can dispatch it like so (to test mediator integration - assume this is SomeViewMediatorTest)

   testView.loginClickedSignal.dispatch(USERNAME, PASSWORD);
   

   if the signal is locked inside the view then you have to create an extending subclass which exposes it in order to fire the signal, which you then use for testing purposes only. I'm not totally averse to test-only classes - I use them a lot - but I do appreciate being able to test signal dispatch integration without having to make a subclass.

   Stray

   Admin Functions

   • Edit
   • Remove

7. 38 Posted by Glen Whitbeck on 08 Feb, 2011 06:32 PM

   

   Hi Stray,

   Thanks for the added detail!

   In your implementation, how are you creating the signals? Is your view instantiating them? Are you letting the RobotLegs context instantiate them (perhaps mapping them as singletons)?

   Also, is your SignalMediator a class that manages all of the signals for your application?

   Admin Functions

   • Edit
   • Split
   • Remove

8. Support Staff 39 Posted by Stray on 08 Feb, 2011 06:54 PM

   

   Hi Glen,

   The SignalMediator isn't for the application, it's per view, what it adds is functionality like the eventMap in a normal Mediator. You just extend SignalMediator instead of the plain Mediator. It just manages the registration / de-registration process so that you don't have to override onRemove in each mediator and remove all your signal handlers manually.

   Perhaps MediatorForViewsWithSignals would be a better name for it? SignalMediator makes it sound like it's more Signal focussed than it actually is.

   Yes - view signals are being created by the views - I don't go for view injection at all. I go for lazy instantiation for signals, so inside the LogInPanelView would be:

   public function get submitClicked():ISignal
   {
       return _submitClicked ||= new Signal(String, String);
   }
   

   The LogInPanelViewMediator would extend SignalMediator instead of just Mediator, and would use the utility function to add the handler for this listener in onRegister, knowing that it'll be automagically removed when the Mediator is removed - triggered by the view leaving the stage.

   As I said before - really I should abstract the actual behaviour in to a SignalMap - identical to the EventMap that is used in the Mediator. It's on the TODO list :)

   Stray

   Admin Functions

   • Edit
   • Remove

9. 40 Posted by Jonny Reeves on 08 Feb, 2011 09:17 PM

   

   Great conversation!

   Stray, I think you nailed it earlier in the thread with the quote:

   In a statically typed language, it's simply not possible to use a notification system of any kind without both parties having appropriate structural knowledge about the message types.

   Unfortunately the same applies for the registration of event / signal handlers. Although we could go ahead and try to create a common interface for registering and removing the notifications dispatched from the View. we can't specify the Type of the notification.
   

   It would strike me that the best approach would be to have a SignalMediator which implements IMediator and contains a SignalMap instead of an EventMap for handling and processing the incoming Signal notifications from the View object. However, until RobotLegs fully 'adopts' signals, this will always remain a 3rd party utility (although it could easily be distributed as a companion SWC).

   Me, personally, I'm happy with Events (:

   Admin Functions

   • Edit
   • Split
   • Remove

10. 41 Posted by Stray on 08 Feb, 2011 09:28 PM

    

    Thanks Jonny!

    I've actually just finished refactoring SignalMediator to use a SignalMap - I just want to add a couple more tests for corner cases but I'll share the code tomorrow as a Solution thread.

    Stray

    Admin Functions

    • Edit
    • Split
    • Remove

11. 42 Posted by Glen Whitbeck on 08 Feb, 2011 09:34 PM

    

    Nice! I like it!

    I guess part of what I was wondering was whether you even needed the signal clean-up, in this case. The mediator's preRemove() method gets called after the view's REMOVED_FROM_STAGE event (it also nulls-out its reference to the view). So, at that point, the view is ~ gone and its locally-instantiated signals should be eligible for garbage collection. If the signals were singletons, they could continue to live-on (and, therefore, continue to hold a reference to the mediator's callback method). Thus, they could keep the mediators from being GC'd.

    The only reason I even went down the path of wondering about whether signal clean-up was necessary was that the need to "clean-up the signal handlers" is really what leads to the need to expose them to the mediator (the view can't do it once it's dead ... so, if the signal can outlive the view, it could hold the mediator hostage). I suppose you could do something on the capture phase of the removal of the view ... but, that might be too much magic!

    I really like what you're doing (and will probably steal your idea!) here! It's hard to argue that it's ever bad to have good clean-up, including removing listeners, etc. (Joel would say "always" have a remove for every add!). In other words, even if you could prove that a particular situation didn't need it, the benefit of leaving it out in that situation is probably not enough to offset the benefits of never forgetting to properly clean-up. I think this is a great idea. For one thing, it allows you (and your team members) to not have to think about whether a signal was created in the view's scope our outside the view's scope (or whether it eventually gets changed without clean-up code being added).

    I think you've sold me on the idea that ~ automatic clean-up is worth the extra demeter "dot" for signals!

    Admin Functions

    • Edit
    • Split
    • Remove

12. 43 Posted by Stray on 08 Feb, 2011 09:47 PM

    

    Excellent!

    I'll share the SignalMap / SignalMediator code on github tomorrow - feel free to give it a good going over and contribute suggestions.

    I'm with Joel - always clean up your mess.

    You're right, in *many* situations the view will clean up after itself... but... say you've got a parent view that is holding this view as a variable. Removing the view from the stage != the view doesn't exist. Often I don't kill my views, I just toggle them on and off the display list so that they maintain their state - if the user has expanded part of a concertina menu or something they don't want to be back to square one if they open that menu again.

    Now your mediator is going to persist because of the signal handlers, and then when your view is added, onRegister runs in the new fresh mediator, and all of a sudden everything is happening TWICE. WTF?

    It can happen pretty easily - the signals don't have to be injected for that situation to arise.

    Awesome that you came round to composition-for-magic-clean-up over single-dot-OCD.

    Peer review FTW :)

    (And thanks again to dj who asked the question!)

    Stray

    Admin Functions

    • Edit
    • Split
    • Remove

13. 44 Posted by Glen Whitbeck on 08 Feb, 2011 10:05 PM

    

    You're totally correct! Great point. And, that's yet another way of how you could think you're safe, but be in for a surprise. And, your solution is so effortless (just extend this mediator and add signals in this manner ... and your signals get cleaned-up automatically).

    Thanks for taking the time to share your technique!

    Admin Functions

    • Edit
    • Split
    • Remove

14. Support Staff 45 Posted by Robert Penner on 09 Feb, 2011 07:23 AM

    

    Re:

    protected function addToSignal(signal:ISignal, handler:Function):void
    

    @Stray, are you doing this because ISignal currently doesn't expose removeAll()? Because if we changed that, we could just iterate through a list of ISignals and call removeAll() on them.

    Admin Functions

    • Edit
    • Remove

15. Support Staff 46 Posted by Stray on 09 Feb, 2011 09:04 AM

    

    Hi Robert,

    I think that would assume that the handlers in this mediator are the only ones at the party for this ISignal?

    In my experience that's not always the case - sometimes my views also have a parent view listening for this signal.

    In the case of signals injected in to the mediator I think we could definitely assume that this mediator isn't the only one registered.

    We only want to unmap the handlers that were added in the onRegister of this mediator, so I think for a utility that is on a par with EventMap we need to track the handlers in the SignalMap for each mediator rather than do a removeAll().

    Have I missed something?

    In the RelaxedEventMap I provide a removeHandlersFor(obj) function, but that relies on tracking the object that added the function by passing it as an extra param when you register. I think that would be unnecessary drag on the Signal classes, as the majority of uses probably don't require it?

    There is that trick of creating an Error and then parsing it to find out the object that owns a function but the overhead on that is even higher.

    Maybe SignalMap belongs with the as3_signals library?

    Stray

    Admin Functions

    • Edit
    • Remove

16. Support Staff 47 Posted by Robert Penner on 09 Feb, 2011 11:56 AM

    

    Oh right... I get it.

    On Feb 9, 2011 1:05 AM, "Stray" <***@tenderapp.com> wrote:
    > // Add your reply above here
    > ==================================================
    > From: Stray
    > Subject: Best practice... want a reusable parent view mediator to be aware of things happening in a resuable child view?
    >
    > Hi Robert,
    >
    > I think that would assume that the handlers in this mediator are the only ones at the party for this ISignal?
    >
    > In my experience that's not always the case - sometimes my views also have a parent view listening for this signal.
    >
    > In the case of signals injected in to the mediator I think we could definitely assume that this mediator isn't the only one registered.
    >
    > We only want to unmap the handlers that were added in the onRegister of this mediator, so I think for a utility that is on a par with EventMap we need to track the handlers in the SignalMap for each mediator rather than do a removeAll().
    >
    > Have I missed something?
    >
    > In the RelaxedEventMap I provide a removeHandlersFor(obj) function, but that relies on tracking the object that added the function by passing it as an extra param when you register. I think that would be unnecessary drag on the Signal classes, as the majority of uses probably don't require it?
    >
    > There is that trick of creating an Error and then parsing it to find out the object that owns a function but the overhead on that is even higher.
    >
    > Maybe SignalMap belongs with the as3_signals library?
    >
    > Stray
    >
    > View this Discussion online: http://knowledge.robotlegs.org/discussions/questions/109-best-practice-want-a-reusable-parent-view-mediator-to-be-aware-of-things-happening-in-a-resuable-child-view
    > --
    >
    > Reply with #ignore to stop receiving notifications for this discussion.
    >
    

    Admin Functions

    • Edit
    • Remove

17. Stray closed this discussion on 16 Feb, 2011 09:11 PM.

Comments are currently closed for this discussion. You can start a new one.