Method chaining + inheritance don't play well together?

Question

Consider:

// member data omitted for brevity

// assume that "setAngle" needs to be implemented separately
// in Label and Image, and that Button does need to inherit
// Label, rather than, say, contain one (etc)

struct Widget {
    Widget& move(Point newPos) { pos = newPos; return *this; }
};

struct Label : Widget {
    Label& setText(string const& newText) { text = newText; return *this; }
    Label& setAngle(double newAngle) { angle = newAngle; return *this; }
};

struct Button : Label {
    Button& setAngle(double newAngle) {
        backgroundImage.setAngle(newAngle);
        Label::setAngle(newAngle);
        return *this;
    }
};

int main() {
    Button btn;

    // oops: Widget::setText doesn't exist
    btn.move(Point(0,0)).setText("Hey");

    // oops: calling Label::setAngle rather than Button::setAngle
    btn.setText("Boo").setAngle(.5); 
}

Any techniques to get around these problems?

Example: using template magic to make Button::move return Button& or something.

edit It has become clear that the second problem is solved by making setAngle virtual.

But the first problem remains unsolved in a reasonable fashion!

edit: Well, I guess it's impossible to do properly in C++. Thanks for the efforts anyhow.

Answer 1

Really, method chaining is a bad idea due to poor exception safety. Do you really need it?

Answer 2

I'd abandon the chaining stuff. For one, it can't actually be done without doing some relatively nasty hacks. But, the biggest problem is that it makes it harder to read and maintain code and you will most likely end up with people abusing it, creating one giant line of code to do multiple things (think back to high-school algebra and those gigantic lines of additions, subtractions and multiplications you always seem to end up with at some point, that is what people will do if you let them).

Another problem is that because most of your functions in the system are going to be returning a reference to itself, its going to be logical that all of them should. When (not if) you finally do start implementing functions that should return values as well (not just accessors, but some mutators will as well, and other generic functions), you will be faced with a dilemma, either break your convention (which will snowball, making it unclear as to how things should be implemented for other future functions) or be forced to start returning values via parameters (which I'm sure you would loathe, as most other programmers I know do as well).

Answer 3

Other people have hit on design issues. You can sort of workaround the problem (albeit in a pretty gross fashion) using C++'s support for covariant return types.

struct Widget {
    virtual Widget& move(Point newPos) { pos = newPos; return *this; }
};

struct Label : Widget {
    Label& move(Point newPos) { pos = newPos; return *this; }
    Label& setText(string const& newText) { text = newText; return *this; }
    Label& setAngle(double newAngle) { angle = newAngle; return *this; }
};

struct Button : Label {
    Button& setAngle(double newAngle) {
        backgroundImage.setAngle(newAngle);
        Label::setAngle(newAngle);
        return *this;
    }
};

int main() {
    Button btn;

    // oops: Widget::setText doesn't exist
    btn.move(Point(0,0)).setText("Hey");

    // oops: calling Label::setAngle rather than Button::setAngle
    btn.setText("Boo").setAngle(.5); 
}

Really though chaining methods the way you are makes for strange code, and hurts readability rather than helps. If you killed the return reference to self junk your code would become:

struct Widget {
    void move(Point newPos) { pos = newPos; }
};

struct Label : Widget {
    void setText(string const& newText) { text = newText; }
    void setAngle(double newAngle) { angle = newAngle; }
};

struct Button : Label {
    void setAngle(double newAngle) {
        backgroundImage.setAngle(newAngle);
        Label::setAngle(newAngle);
    }
};

int main() {
    Button btn;

    // oops: Widget::setText doesn't exist
    btn.move(Point(0,0));
    btn.setText("Hey");

    // oops: calling Label::setAngle rather than Button::setAngle
    btn.setText("Boo");
    btn.setAngle(.5); 
}

Answer 4

You can extend CRTP to handle this. monjardin's solution goes in the right direction. All you need now is a default Label implementation to use it as a leaf class.

#include <iostream>

template <typename Q, typename T>
struct Default {
    typedef Q type;
};

template <typename T>
struct Default<void, T> {
    typedef T type;
};

template <typename T>
void show(char const* action) {
    std::cout << typeid(T).name() << ": " << action << std::endl;
}

template <typename T>
struct Widget {
    typedef typename Default<T, Widget<void> >::type type;
    type& move() {
        show<type>("move");
        return static_cast<type&>(*this);
    }
};

template <typename T = void>
struct Label : Widget<Label<T> > {
    typedef typename Default<T, Widget<Label<T> > >::type type;
    type& set_text() {
        show<type>("set_text");
        return static_cast<type&>(*this);
    }
};

template <typename T = void>
struct Button : Label<Button<T> > {
    typedef typename Default<T, Label<Button<T> > >::type type;
    type& push() {
        show<type>("push");
        return static_cast<type&>(*this);
    }
};

int main() {
    Label<> lbl;
    Button<> btt;

    lbl.move().set_text();
    btt.move().set_text().push();
}

That said, consider whether such an effort is worth the small added syntax bonus. Consider alternative solutions.

Answer 5

For a while there I thought that it might be possible to overload the slightly unusual operator->() for chaining methods instead of ., but that faltered because it seems the compiler requires the identifier to the right of the -> to belong to the static type of the expression on the left. Fair enough.

Poor Man's Method Chaining

Stepping back for a moment, the point of method chaining is to avoid typing out long object names repeatedly. I'll suggest the following quick and dirty approach:

Instead of the "longhand form":

btn.move(Point(0,0)); btn.setText("Hey");

You can write:

{Button& _=btn; _.move(Point(0,0)); _.setText("Hey");}

No, it's not as succinct as real chaining with ., but it will save some typing when there are many parameters to set, and it does have the benefit that it requires no code changes to your existing classes. Because you wrap the entire group of method calls in {} to limit the scope of the reference, you can always use the same short identifier (e.g. _ or x) to stand for the particular object name, potentially increasing readability. Finally, the compiler will have no trouble optimising away _.

Answer 6

Not with C++.

C++ does not support variance in return types, so there's no way to change the static type of the reference returned from Widget.move() to be more specific than Widget& even if you override it.

The C++ needs to be able to check things at compile time, so you can't use the fact that what's really being returned from move is a button.

At best, you can do some runtime casting, but it's not going to look pretty. Just separate calls.

Edit: Yes, I'm well aware of the fact that the C++ standard says that return value covariance is legitimate. However, at the time I was teaching and practicing C++, some mainstream compilers (e.g., VC++) did not. Hence, for portability we recommended against it. It is possible that current compilers have no issue with that, finally.

Answer 7

C++ does support return value covariance on virtual methods. So you could get something like what you want with a little work:

#include <string>
using std::string;

// member data omitted for brevity

// assume that "setAngle" needs to be implemented separately
// in Label and Image, and that Button does need to inherit
// Label, rather than, say, contain one (etc)


struct Point
{
    Point() : x(0), y(0) {};
    Point( int x1, int y1) : x( x1), y( y1) {};

    int x;
    int y;
};

struct Widget {
    virtual Widget& move(Point newPos) { pos = newPos; return *this; }
    virtual ~Widget() {};

    Point pos;
};

struct Label : Widget {
    virtual ~Label() {};
    virtual Label& move( Point newPos) { Widget::move( newPos); return *this; }

    // made settext() virtual, as it seems like something 
    // you might want to be able to override
    // even though you aren't just yet
    virtual Label& setText(string const& newText) { text = newText; return *this; }
    virtual Label& setAngle(double newAngle) { angle = newAngle; return *this; }

    string text;
    double angle;
};

struct Button : Label {
    virtual ~Button() {};
    virtual Button& move( Point newPos) { Label::move( newPos); return *this; }
    virtual Button& setAngle(double newAngle) {
        //backgroundImage.setAngle(newAngle);
        Label::setAngle(newAngle);
        return *this;
    }
};

int main()
{
    Button btn;

    // this works now
    btn.move(Point(0,0)).setText("Hey");

    // this works now, too
    btn.setText("Boo").setAngle(.5); 

  return 0;
}

Note that you should use virtual methods for doing something like this. If they aren't virtual methods, then the 'reimplemented' methods will result in name-hiding and the method called will depend on the static type of the variable, pointer or reference so it might not be he correct method if a base pointer or reference is being used.

Answer 8

Do setText() and setAngle() really need to return their own types in each class? If you set them all to return Widget&, then you can just use virtual functions as follows:

struct Widget {
    Widget& move(Point newPos) { pos = newPos; return *this; }
    virtual Widget& setText(string const& newText) = 0;
    virtual Widget& setAngle(double newAngle) = 0;
};

struct Label : Widget {
    virtual Widget& setText(string const& newText) { text = newText; return *this; }
    virtual Widget& setAngle(double newAngle) { angle = newAngle; return *this; }
};

struct Button : Label {
    virtual Widget& setAngle(double newAngle) {
        backgroundImage.setAngle(newAngle);
        Label::setAngle(newAngle);
        return *this;
    }
};

Note that even if the return type is Widget&, the Button- or Label-level functions will still be the ones called.

Answer 9

Is a Button really a Label? You seem to be violating the Liskov substitution principle. Perhaps you should consider the Decorator pattern to add behaviors to Widgets.

If you insist on the structure as is, you can solve your problem like so:

struct Widget {
    Widget& move(Point newPos) { pos = newPos; return *this; }
    virtual ~Widget();  // defined out-of-line to guarantee vtable
};

struct Label : Widget {
    Label& setText(string const& newText) { text = newText; return *this; }
    virtual Label& setAngle(double newAngle) { angle = newAngle; return *this; }
};

struct Button : Label {
    virtual Label& setAngle(double newAngle) {
        backgroundImage.setAngle(newAngle);
        Label::setAngle(newAngle);
        return *this;
    }
};

int main() {
    Button btn;

    // Make calls in order from most-specific to least-specific classes
    btn.setText("Hey").move(Point(0,0));

    // If you want polymorphic behavior, use virtual functions.
    // Anything that is allowed to be overridden in subclasses should
    // be virtual.
    btn.setText("Boo").setAngle(.5); 
}

Answer 10

I think (I haven't tested it) this will do it using templates:

template<class T> struct TWidget {
    T& move(Point newPos) { pos = newPos; return (T&)*this; }
};

template<class T> struct TLabel : TWidget<T> { ... }

struct Label : TLabel<Label> { ... }

struct Button : TLabel<Button> { ... }

Notes:

• Any/every base class needs to be a template, with a separate non-template leaf class at the top (contrast the LabelT and Label classes).
• The cast could be a dynamic_cast if you like.
• Instead of casting the "return *this", the base class could contain a T& as a data member (the derived class would pass this to the base class' constructor), which would be an extra data member, but which avoids a cast and I think may permit composition instead of or as well as inheritance.

Answer 11

A simple, but annoying, way of solving your problem is to reimplement all your public methods in your subclasses. This doesn't solve the issue with polymorphism (if you cast from Label to Widget, for example), which may or may not be a major issue.

struct Widget {
    Widget& move(Point newPos) { pos = newPos; return *this; }
};

struct Label : Widget {
    Label& setText(string const& newText) { text = newText; return *this; }
    Label& setAngle(double newAngle) { angle = newAngle; return *this; }
    Label& move(Point newPos) { Widget::move(newPos); return *this; }
};

struct Button : Label {
    Button& setText(string const& newText) { Label::setText(newText); return *this; }
    Button& setAngle(double newAngle) {
        backgroundImage.setAngle(newAngle);
        Label::setAngle(newAngle);
        return *this;
    }
    Button& move(Point newPos) { Label::move(newPos); return *this; }
};

Be sure to include in your documentation this must be done for chaining to work.

But really, now, why bother with method chaining? Many times, these functions will be called less trivially, and will need longer lines. This would really hurt readability. One action per line -- this is a general rule when it comes to ++ and -- too.

Answer 12

This compiles on gcc 4.3.2 and is sort of a mixin pattern.

#include <string>

using namespace std;

struct Point {
    Point() : x(0), y(0) {}
    Point(int x, int y) : x(x), y(y) {}

    int x, y;
};

template <typename T>
struct Widget {
    T& move(Point newPos) {
        pos = newPos;
        return *reinterpret_cast<T *> (this);
    }

    Point pos;
};

template <typename T>
struct Label : Widget<Label<T> > {
    T& setText(string const& newText) {
        text = newText;
        return *reinterpret_cast<T *> (this);
    }
    T& setAngle(double newAngle) {
        angle = newAngle;
        return *reinterpret_cast<T *> (this);
    }

    string text;
    double angle;
};

struct Button : Label<Button> {
    Button& setAngle(double newAngle) {
        backgroundImage.setAngle(newAngle);
        Label<Button>::setAngle(newAngle);
        return *this;
    }

    Label<Button> backgroundImage;
};

int main() {
    Button btn;

    // oops: Widget::setText doesn't exist
    btn.move(Point(0,0)).setText("Hey");

    // oops: calling Label::setAngle rather than Button::setAngle
    btn.setText("Boo").setAngle(0.0); 
}

Answer 13

For the second problem, making setAngle virtual should do the trick.

For the first one, there are no easy solutions. Widget::move returns a Widget, which doesn't have a setText method. You could make a pure virtual setText method, but that'd be a pretty ugly solution. You could overload move() on the button class, but that'd be a pain to maintain. Finally, you could probably do something with templates. Perhaps something like this:

// Define a move helper function
template <typename T>
T& move(T& obj, Point& p){ return obj.move(p); };

// And the problematic line in your code would then look like this:
move(btn, Point(0,0)).setText("Hey");

I'll let you decide which solution is cleanest. But is there any particular reason why you need to be able to chain these methods?

Answer 14

Well, you know it's a Button so you should be able to cast the returned Widget& as a Button& and keep going. It does look a bit ugly though.

Another rather annoying option is to create a wrapper in your Button class for the Widget::move function (and friends). Probably not worth the effort to wrap everything if you have more than a handful of functions though.

Answer 15

[rant]

Yes. Quit this method chaining business and just call functions in a row.

Seriously, you pay a price for allowing this syntax, and I don't get the benefits it offers.

[/rant]