views:

67

answers:

4

I have a pretty simple scenario here and it works, but my instincts tell me I'm making a mistake and would like to run it by some better minds.

Here I have a parent class that instantiates a MenuClass and handles transitions between pages.

public class ParentClass extends Sprite
{
    public function ParentClass()
    {
        setupOptionMenu();
    }

    private function setupOptionMenu() : void
    {
        var myMenu:MenuClass = new MenuClass;
        myMenu.setUpButtons();
        this.addChild( myMenu );
    }

    + public function transitionForward() : void

    + public function transitionBackward() : void
}

And here's the MenuClass which creates forward and backward buttons. Clicking each will tell the above ParentClass to transitionForward() or transitionBackward() respectively.

public class MenuClass extends Sprite
{
    public function MenuClass()
    {
        setupButtons();
    }

    public function setUpButtons() : void
    {
        var backButton:Button = new Button();
        backButton.addEventListener( MouseEvent.CLICK, backClick );
        addChild( backButton );

        var forwardButton:Button = new Button();
        forwardButton.addEventListener( MouseEvent.CLICK, forwardClick );
        addChild( forwardButton );
    }

    private function backClick( e:Event ) : void
    {
        ParentClass( this.parent ).transitionForward();
    }

    private function forwardClick( e:Event ) : void
    {
        ParentClass( this.parent ).transitionBackward();
    }
}

One one hand, I feel like the MenuClass is too tightly bound to its parent and is thus less reusable. On the other hand I feel like, in the interest of keeping everything self contained, it's not the ParentClass's responsibility to reach into the MenuClass instance and add event listeners there. This seems like there should be some kind of OOP rule-of-thumb to govern this kind of relationship.

So my question is: Is my MenuClass too tightly bound to its parent? And if so, do you have any suggestions that would loosen that coupling?

Thanks.

+1  A: 

On the other hand I feel like, in the interest of keeping everything self contained, it's not the ParentClass's responsibility to reach into the MenuClass instance and add event listeners there.

Why not? If the ParentClass needs to listen for events on MenuClass, then it should be adding those event listeners itself. Either by calling addEventListener directly, or calling another method on the MenuClass that sets up the event handlers with the right function to call.

Anon.
A: 

IMHO you can do that if your application is a small/medium one, for bigger projects you are right on decoupling the parent class and the child class. There are several approaches based on taste, one would be to create to use a "FlowControl" class.

public class FlowControl
{
   private var _parentSprite : ParentClass;
   private var _instance : FlowControl;

   public static FlowControl GetInstance(ParentSprite : ParentClass)
   {

        if (_instance == null)
            _instance = new FlowControl(ParentSprite);
        return _instance;

   }

   private FlowControl(ParentSprite : ParentClass)
   {
       _parentSprite = ParentSprite;
   }

   public static void NextPage()
   {
       ParentSprite.transitionForward();
   }
}

I don't have flash to test it out, but you get the idea.

A better solution would be to implement an interface, so you get a tight coupling on an interface and not a class.

jpabluz
A: 

It's not "bad" form, but depending on the context there might be other ways of approaching it.

Especially if you have multiple children with a single parent.

I am a big fan of static methods and Manager classes, but there are a whole cohort of people who are equally against that or worse as opposed to child->parent method calls.

The big thing to note is that "parent" is a property that hinges upon addChild() and the display list.

I might be wrong but it is my understanding that if you create an object in code, and it is never added to a display list in its lifecycle, then parent will be null. So that presents issues.

The other quirk is that if you add a child to the display list, and then REMOVE it, parent is still set to the last object it was added to, so it may be able to take action against an object it is technically no longer a child of.

This is why I usually phrase this logic as follows

if(Type(this.parent).contains(this))
{
//do stuff
}

Contains will only return true is the object is presently in the display list.

Some situations where this type of logic might be relevant is if you are looping on array of objects where some objects may be visible and others are not. At worst it will save you some CPU cycles.

Jasconius
+4  A: 

If there's any chance you'll want to offer multiple ways to navigate (besides just the menu) then using an event-driven approach makes sense. It lets you almost completely decouple the Parent from the Menu.

Here's some code I actually tested in Flash. ;)

The parent creates the menu object and listens for FORWARD and BACKWARD events from it:

package  
{
        import flash.display.Sprite;
        import flash.events.Event;

        public class ParentClass extends Sprite
        {
            public function ParentClass() 
            {
                setupOptionMenu();
            }

            private function setupOptionMenu() : void
            {
                var myMenu:MenuClass = new MenuClass;
                myMenu.setUpButtons();
                this.addChild( myMenu );

                myMenu.addEventListener(MenuClass.FORWARD, transitionForward);
                myMenu.addEventListener(MenuClass.BACKWARD, transitionBackward);
            }

            public function transitionForward(e:Event) : void
            { 
                trace("forward");
            }

            public function transitionBackward(e:Event) : void
            {
                trace("backward");
            }
        }
}

Then the MenuClass dispatches these events when something is clicked:

package  
{
        import flash.display.Sprite;
        import flash.events.Event;
        import flash.events.MouseEvent;
        import flash.display.SimpleButton;

        public class MenuClass extends Sprite
        {
            public static const FORWARD:String = "forward";
            public static const BACKWARD:String = "backward";

            public function MenuClass() 
            {
                setUpButtons();
            }

            public function setUpButtons() : void
            {
                var rect:Sprite = new Sprite();
                rect.graphics.beginFill(0x666666);
                rect.graphics.drawRect(0,0,50,20);

                var backButton:SimpleButton = new SimpleButton(rect, rect, rect, rect);
                backButton.x = 10;
                backButton.y = 10;
                backButton.addEventListener( MouseEvent.CLICK, backClick );
                addChild( backButton );

                var forwardButton:SimpleButton = new SimpleButton(rect, rect, rect, rect);
                forwardButton.x = 70;
                forwardButton.y = 10;
                forwardButton.addEventListener( MouseEvent.CLICK, forwardClick );
                addChild( forwardButton );
            }

            private function backClick( e:MouseEvent ) : void
            {
                dispatchEvent(new Event(BACKWARD));
            }

            private function forwardClick( e:MouseEvent ) : void
            {
                dispatchEvent(new Event(FORWARD));
            }
        }
}

To provide alternate UI methods, like keyboard shortcuts or context menus, etc. just have dispatch the same FORWARD and BACKWARD events and have the Parent listen for them too. You could also create a new Event subclass and put the FORWARD and BACKWARD constants in there if you want.

RobDixon
this is how I do it. This way the child need not know anything about the outside world and thus enforcing encapsulation and loose coupling.
Allan
+1. clean and simple. :)
back2dos
@RobDixon: I'm curious about this: doesn't it make things MORE coupled? Now the Parent needs to know about (ie maintain listeners for) every possible source of FORWARD and BACKWARD events. Why not just let the child (or any other UI element) call a forward() or backward() method on the Parent?
Richard Inglis
@Richard Inglis: nope. it definitely doesn't make them MORE coupled. before, ParentClass was depending on MenuClass and MenuClass on ParentClass. Now, we have a one-way-dependancy. and this is the wrong place for dependency inversion, if that's what you were trying to say. ParentClass clearly uses MenuClass for composition, thus the dependancy is perfectly valid. it could be reduced even more by using event bubbling, but then again I find event bubbling not to be a very robust way of message passing.
back2dos