8

I have a task to implement a simple SVG generator. I need to support Circle, Polyline and Text. All three have at least 4 common methods: - SetStrokeColor - SetFillColor - SetStrokeWidth - ToString One of the main requirements is to support chaining, e.g.: Polyline{}.SetStrokeColor("white").SetFillColor("black")...

I decided to implement a base class Element, which all the other classes inherit from. The idea is to have a class Document that holds a vector of all the elements added to the document. A sample signature for a base method:

// source of trouble
Element &SetStrokeColor(const Color &color) {
    ...
    return *this;
}

My derived classes do call these methods, but the trouble is that the methods return a reference to the base class Element, not the derived class.

My question is whether it is all together possible to implement in c++???

Further discussion here

magom001
  • 608
  • 6
  • 16
  • 1
    Please take some time to refresh [the help pages](http://stackoverflow.com/help), retake [the SO tour](http://stackoverflow.com/tour), and reread about [how to ask good questions](http://stackoverflow.com/help/how-to-ask), as well as [this question checklist](https://codeblog.jonskeet.uk/2012/11/24/stack-overflow-question-checklist/). Lastly please don't forget how to create a [mcve]. – Some programmer dude Jun 03 '19 at 08:03
  • Not a problem at all - solely that *your* signature has a surplus ampersand... – Aconcagua Jun 03 '19 at 08:06
  • @Aconcagua typo – magom001 Jun 03 '19 at 08:10
  • a reference to the base is a an abstract reference to the derived class. what exactly do you want to achieve? given that you want to store all elements in a vector, you only have access to `Element` later anyways – local-ninja Jun 03 '19 at 08:14
  • Typo - sure; just be aware that it still produced valid c++ and you would have returned an rvalue reference... – Aconcagua Jun 03 '19 at 08:15
  • @fdan chaining: Circle{}.SetStrokeWidth(16).SetCircleCenter({0, 0}). SetStrokeWidth returns a reference to Element, so SetCircleCenter is unavailable. – magom001 Jun 03 '19 at 08:16
  • @magom001 covariants won't make you happy in this case. this will only work as long as you know what type an element is (and please, do not upcast). the information is lost once you store the cirle as an element. if you go for CRTP you can't store the elements in homogeneous lists. I would drop the idea of chaining in base classes (if you need to access non-base functions) – local-ninja Jun 03 '19 at 08:43

3 Answers3

9

If you want to share implementations and preserve type information, the CRTP is what you need:

struct ElementBase { };

template <class Concrete>
struct Element : ElementBase {

    Concrete &setStrokeWidth(int width) {
        // Actual implementation...
        (void) width;

        return cthis();
    }

private:
    friend Concrete;
    Element() = default;

    Concrete &cthis() { return static_cast<Concrete &>(*this); }
    Concrete &cthis() const { return static_cast<Concrete const &>(*this); }
};

struct Circle : Element<Circle> {
    Circle &SetCircleCenter(int x, int y) {
        // Actual implementation...
        (void) x;
        (void) y;

        return *this;
    }
};

int main() {
    Circle c;
    c.setStrokeWidth(4).SetCircleCenter(0, 0);
}

See it live on Wandbox

Quentin
  • 62,093
  • 7
  • 131
  • 191
  • one little thing missing: how do I put objects of type Element into a vector>? – magom001 Jun 03 '19 at 17:36
  • found an answer: derived Element from a non template Base class. Make unique_ptr. Would you mind adding it to your solution, so that others would not have to read through the comments? – magom001 Jun 03 '19 at 18:04
  • @magom001 certainly, but do note that, due to the issue described in lubgr's answer, you won't be able to call the functions dynamically from the base class. It can be circumvented, though, if you need that. – Quentin Jun 03 '19 at 18:38
  • sure, in my particular case I only need to call a ToString method which I declare pure virtual in the Base class and override in subclasses. That's what chaining was intended for: you call Svg::Document::Add(Svg::Circle{}.SetCenter({0,0}).SetRadius(2)), the Add method does vector.emplace_back(make_unique(circle)) When I need to generate an svg I just iterate over all the elements in vector and call ToString on each. – magom001 Jun 03 '19 at 19:26
4

With covariant return types, you can

class Element {
  public:
    // ...

    virtual Element& refToThis() { return *this; };
};

and in derived classes

class Derived : public Element {
  public:
    // ...

    Derived& refToThis() override { return *this; };
};

which lets you handle Derived instances as Derived instances when the static type is Derived (e.g. inside Derived itself). When tht static type is Element, the return type of refToThis() is, too.

lubgr
  • 37,368
  • 3
  • 66
  • 117
  • Thanks for the answer! I still don't get it. The base method not just simple returns a ref. It does some logic etc. Does it mean that I have to reimplement the method for each derived class? – magom001 Jun 03 '19 at 08:09
  • Yes, if you want the derived type to be available through `SomeDerivedClass::refToThis()`, then every subclass has to implement this member function. – lubgr Jun 03 '19 at 08:32
1

For comparison:

class Base {};
class Derived : public Base {};

Derived d;
Base* p = &d; // points to a Base that in reality is a derived
Base& b =  d; // in this respect, references do not differ...

// and you can get the original type back:
auto dr = static_cast<Derived&>(b);  // if you know 100% for sure that b is a Derived
auto dp = dynamic_cast<Derived*>(p); // if p might point to another type; you get
                                     // a null pointer back, if it does

There is absolutely no difference with returning pointers or references to this/*this, so yes, you can safely do so.

Edit:

Circle().SetStrokeWidth(16).SetCircleCenter({0, 0}). SetStrokeWidth returns a reference to Element, so SetCircleCenter is unavailable.

In this case, you are a bit in trouble, though. As lubgr denoted already, you can solve the issue by overriding with co-variant return type – and yes, this would mean that you'd have to override each function separately. Alternatively, you can save all this hassle using CRTP:

template <typename T>
class SomeAppropriateName : public Element
{
public:
    T& setStrokeWidth(unsigned int width)
    {
        Element::setStrokeWidth(width);
        return static_cast<T&>(*this);
    }
};

class Circle : public SomeAppropriateName<Circle>
{
    // the overrides needed are inherited...
};

For this approach, original functions from Element class need to be non-virtual (thanks @Quentin for the hint; actually an advantage as virtual function calls are more costly...); in the template, the argument (T, i. e. Circle) is not yet completely defined (with CRTP pattern) and the compiler wouldn't be able to check if the return type really is co-variant. So actually, we are just shadowing the base class' function.

This is still no disadvantage compared to overriding: The co-variant return type will only be available if the (virtual) function is called on the object directly anyway:

Circle c;
c.setStrokeWidth(7).setCenter();

Element& e = c;
e.setStrokeWidth(7).setCenter(); // fails with both variants, i.e. even if you
                                 // implement co-variant override
Aconcagua
  • 24,880
  • 4
  • 34
  • 59
  • This, alas, does not work, since `Circle` (aka `T`) is not complete when `setStrokeWidth` is declared, and its inheritance graph is unknown. Thus the covariance of the return types cannot be verified and the code does not compile. – Quentin Jun 03 '19 at 08:42
  • Still no dice (I stumbled into it while writing my answer...) -- even without an explicit `override` the signatures match so the check is performed. I think that's only solvable by using another name (or signature) for the function, *à la* NVI. Here's a [demo of the issue](https://wandbox.org/permlink/XJEiYrHTaoYXxMRR). – Quentin Jun 03 '19 at 09:13
  • Edit: hold up, *your* code works on Clang as well. What did I do wrong... – Quentin Jun 03 '19 at 09:22
  • 1
    Got it: your base class function is *not* `virtual`. I'm not crazy yet, yay! – Quentin Jun 03 '19 at 09:26
  • Just add `virtual` to `Element::setStrokeWidth` in your demo and you'll see it :) – Quentin Jun 03 '19 at 09:34