1

Possible duplicate of:

Consider the following example:

#include <iostream>
#include <cassert>

struct MyEnum {
    enum valid { CASE1, CASE2, DEFAULT };
    valid value;
    MyEnum(valid value = DEFAULT) : value(value) {}
};

class Base {
protected:
    MyEnum value;

public:
    Base() = default;
    Base(MyEnum value) : value(value) {
        std::cout << "Base::Base(MyEnum).\n";
    }  
};

class ReadableBase : public virtual Base {
public:
    ReadableBase() = default;
    ReadableBase(MyEnum value) : Base(value) {
        std::cout << "ReadableBase::ReadableBase(MyEnum).\n";
    }

public:
    MyEnum read() { return value; }
};

class WriteableBase : public virtual Base {
public: 
    WriteableBase() = default;
    WriteableBase(MyEnum value) : Base(value) {
        std::cout << "WriteableBase::WriteableBase(MyEnum).\n";    
    }

public:
    void write(MyEnum value) { this->value = value; }
};

class ReadWriteBase : public ReadableBase, 
                      public WriteableBase {
public:
    ReadWriteBase() = default;
    ReadWriteBase(MyEnum value) : ReadableBase(value), WriteableBase(value) {}
};

int main(int, char*[]) {
    // Incorrectly initialised with value = MyEnum::valid::DEFAULT
    ReadWriteBase rw(MyEnum::valid::CASE1);
    
    // Everything is okay now.
    rw.write(MyEnum::valid::CASE2);
}

As demonstrated, despite the ReadWriteBase derived class accepting the MyEnum with the equivalent value of 0 (= MyEnum::valid::CASE1), the program reports that the value of ReadableBase::value and WriteableBase::value is 2 (= MyEnum::valid::DEFAULT). This appears to be because Base::Base(MyEnum) is not called at all.

assert(rw.ReadableBase::value  == MyEnum::valid::DEFAULT); // true
assert(rw.WriteableBase::value == MyEnum::valid::DEFAULT); // true
assert(rw.Base::value          == MyEnum::valid::DEFAULT); // true

Why does this happen? How do I do this correctly?

Bonus question: Is there a better way to approach this problem?

buzzysin
  • 311
  • 3
  • 12
  • 5
    If you change `Base() = default;` to `Base() = delete;`, it would allow the compiler to help guide you to the pit of success. – Eljay Apr 14 '23 at 11:36
  • 2
    A virtual base is initialised before other bases, and not initialised again. So your constructor of `ReadWriteBase` is equivalent to `ReadWriteBase(MyEnum value) : Base(), ReadableBase(value), WriteableBase(value) {}` and the initialisations of `Base` performed by constructor initialiser lists of `WriteableBase` and `ReadableBase` are both ignored. To do what you want, depending on other needs, your choices include `ReadWriteBase(MyEnum value) : Base(value), ReadableBase(value), WriteableBase(value) {}` or `ReadWriteBase(MyEnum value) : Base(value), ReadableBase(), WriteableBase() {}`. – Peter Apr 14 '23 at 11:49
  • Virtual inheritance behaves like this to stop (among other things) multiple initializations of a base class member with different values. What would you want to happen if you called the base class constructors with different enumerators? (Still looking for a good duplicate for this...) – Adrian Mole Apr 14 '23 at 12:01
  • @AdrianMole I've also been looking for a good dup for this one, but (so far) it seems the well is dry. – Peter Apr 14 '23 at 12:13
  • In general : try to redesign your class hierarchy to NOT require diamond inheritance. But create different classes for different abilities like reading and writing and aggregate them when you need to combine them. "Whatever you may think, in OO inheritance is rarely the solution". The only thing I use it for (and use it a lot is to define interfaces (abstract baseclasses), so I can implement dependency injection. – Pepijn Kramer Apr 14 '23 at 15:06
  • (I can only tag one user per comment) I tried to find a similar question but they don't quite hit the nail on my proverbial head. @Pepijn Kramer Aggregation looks like the way to go. I may just define the methods I need on the base and aggregate the readable and writable contracts. – buzzysin Apr 14 '23 at 15:27

1 Answers1

8

Why does this happen? How do I do this correctly?

virtual base should be initialized in the most derived class, so it should be

class ReadWriteBase : public ReadableBase, 
                      public WriteableBase {
public:
    ReadWriteBase() = default;
    ReadWriteBase(MyEnum value) :
        Base(value),
        ReadableBase(value), // possibly ReadableBase()
        WriteableBase(value) // possibly WriteableBase()
    {}
};
Jarod42
  • 203,559
  • 14
  • 181
  • 302