Request for inheritance design suggestion


#1

I have a MyButton class derived from TextButton, that overrides its mouseUp(), mouseDown() and clicked() methods to do what it needs, like:

class MyButton : public TextButton
{
     void mouseUp (const MouseEvent& e) override { /*do something*/ }
     void mouseDown (const MouseEvent& e) override { /* do something else*/ }
     void clicked() override { /* do stuff*/ }
}

Now, I need to implement the same exact behavior, but in a MyOtherButton class that is derived from DrawableButton.

The easiest way to do that would be to simply copy and paste the implementation of those methods over to the new class, like:

class MyOtherButton : public DrawableButton
{
     void mouseUp (const MouseEvent& e) override { /*do something*/ }
     void mouseDown (const MouseEvent& e) override { /* do something else*/ }
     void clicked() override { /* do stuff*/ }
}

but this feels wrong because it isn’t much D.R.Y., and also because if I have to change some of the implemented behavior in the future, I wish the change to be consistent in both classes. With this design, the only way to keep them in sync requires me to remember to apply the same changes to both, which I don’t like very much.

Is there some clean solution for this that I have overlooked?


#2

What about a new class, that is a Button::Listener AND a MouseListener?

class ButtonFeedback : public Button::Listener, public MouseListener
{
public:
    void mouseUp (const MouseEvent &event) override { /* do something */ }
    void mouseDown (const MouseEvent& e) override { /* do something else*/ }
    void buttonClicked (Button *) override { /* do stuff */ }
}

ScopedPointer<ButtonFeedback> feedback = new ButtonFeedback;
myButton.addListener (feedback);
myButton.addMouseListener (feedback);

Or you can even add ButtonFeedback as a member of your button aggregating the behaviour you want…

class MyButton : public TextButton
{
public:
    MyButton ()
    {
        feedback = new ButtonFeedback;
        addListener (feedback);
        addMouseListener (feedback);
    }
private:
    ScopedPointer<ButtonFeedback> feedback;
}

Or you can even put the addListener into the constructor of the ButtonFeedback:

ButtonFeedback (Button* b)
{
    b->addListener (this);
    b->addMouseListener (this);
}

Just as an idea…


#3

Thanks @daniel, that’s actually something I was considering right now.

Another approach that I was thinking about is somehow similar to the one you proposed, inverting the ownership relation:

class MyButtonDriver : public Button::Listener, public MouseListener
{
public:
    MyButtonDriver (Button* drivenButton) : button (drivenButton) 
    {
         button->addListener (this);
         button->addMouseListener (this);
    }
    
    void mouseUp (const MouseEvent &event) override { /* do something */ }
    void mouseDown (const MouseEvent& e) override { /* do something else*/ }
    void buttonClicked (Button *) override { /* do stuff */ }

private:
    ScopedPointer <Button> button;
}

This way, I don’t even need to create my derived classes for the various types of Buttons, I will simply need to create them and then wrap them into an instance of MyButtonDriver for each, like

DrawableButton* drawableButton = new DrawableButton ( ... );
MyButtonDriver forDrawable (drawableButton);

TextButton* textButton = new TextButton ( ... );
MyButtonDriver forText (textButton);

Does this make sense to your eyes as well?


#4

That’s a good idea…
I like the idea to transfer the ownershit to the MyButtonDriver class… :slight_smile:
I would then add to the MyButtonDriver:

Button* getButton() { return button; }

and create the thing in one line, so you don’t accidently keep and delete the drawableButton, because the ownership is already handled in the MyButtonDriver:

MyButtonDriver forDrawable (new DrawableButton ( ... ));

But if you need to call some other methods on the drawableButton it certainly makes sense not to call getButton() each time…


#5

Yes, the getButton() method is definitely needed.

Perhaps also a way to specify whether the MyButtonDriver should take ownership of the button or just drive it without deleting it on destruction?


#6

Hmm, then you can’t work with the ScopedPointer…
So you need an alternative Button* pointer, that feels wrong to me.
You can do that, but it adds a lot of conditionals in simple tasks.
Or you work with a raw pointer and add a flag, if the pointer shall be destroyed upon destruction of the driver.


#7

Sold, I’ll leave that ownership matter out for the moment, and will add it at a later time if the need arises.

Now I’m tempted to make this a class template that takes the Button subclass as its argument, so that the getButton() method returns the correct class and I won’t need to dynamic_cast it.


#8

You could also template the base class and use the “MyButton” as a skeleton implementation over a button interface, thus solving all problems (?). Until concepts arrive, use SFINAE or a static assert together with std::is_base_of to avoid illegal non-button arguments.