Search code examples
c++inheritancebuilder-pattern

Inheritance with Builder pattern


Let's assume, that we have AbstractBufferBuilder class that contains some common methods for building some Buffer classes:

class AbstractBufferBuilder {
public:

    AbstractBufferBuilder &setBufferData(...) {
        //some code
        return *this;
    }
};

And we have ConcreteBufferBuilder class:


class ConcreteBufferBuilder : public AbstractBuilder {
public:

    ConcreteBufferBuilder &addAttribute(...) {
        //some code
        return *this;
    }

    Buffer* build(){
        return new Buffer(...);
    }
};

To create new Buffer class, we can do

ConcreteBufferBuilder().setBufferData(...).addAttribute(...).build();

But this code won't be compiled because setBufferData() returns AbstractBufferBuilder that doesn't contain addAttribute().

I've tried to use templates to solve this problem:

template<class Builder>
class AbstractBufferBuilder {
public:

    Builder &setBufferData(...) {
        //some code
        return *dynamic_cast<Builder*>(this);
    }
};
class ConcreteBufferBuilder : public AbstractBuilder<ConcreteBufferBuilder> {
public:

    ConcreteBufferBuilder &addAttribute(...) {
        //some code
        return *this;
    }

    Buffer* build(){
        return new Buffer(...);
    }
};

It works, but templates give extra work to the compiler and linker, so it increases build time and file size.

Another possible solution is to use a wrapper for AbstractBufferBuilder methods:


class AbstractBufferBuilder {
protected:

    void setBufferDataImpl(...) {
        //some code
    }
};
class ConcreteBufferBuilder : public AbstractBuilder {
public:
    ConcreteBufferBuilder &setBufferData(...){
        AbstractBuilder::setBufferDataImpl(...);
        return *this;
    }

    ConcreteBufferBuilder &addAttribute(...) {
        //some code
        return *this;
    }

    Buffer* build(){
        return new Buffer(...);
    }
};

But then we need to write same code every time we create a new Builder class which is also not good.

Of course, we can abandon the use of chain calls, but I want to solve this particular problem.

Same question already asked: Builder Pattern and Inheritance, but my question is little bit different.

Is there any better solution to this problem?


Solution

  • I do not have a perfect solution for this problem, but I can suggest some options:

    1. The first one is to stick to the CRTP as you did in your post. In order to minimize compile times and code duplication, the CRTP can be reduced to only the part that performs the casts:
    class AbstractBufferBuilder {
    public:
        virtual ~AbstractBufferBuilder() = default;
    
        AbstractBufferBuilder& setBufferData() {
            // Code does not have to be inline -- can live in a cpp file to diminish compile/link time
            return *this;
        }
    };
    
    template <typename DerivedSelf>
    class AbstractBufferBuilderWrapper : public AbstractBufferBuilder {
    public:
        DerivedSelf& setBufferData() {
            return static_cast<DerivedSelf&>(AbstractBufferBuilder::setBufferData());
        }
    };
    
    class ConcreteBufferBuilder : public AbstractBufferBuilderWrapper<ConcreteBufferBuilder>
    {
    public:
        ConcreteBufferBuilder& addAttribute() {
            return *this;
        }
    
        std::unique_ptr<Buffer> build() {
            return std::make_unique<Buffer>();
        }
    };
    
    int main() {
        auto buffer = ConcreteBufferBuilder {}
            .setBufferData()
            .addAttribute()
            .build();
    }
    
    1. A second option is to drop the CRTP, but to use an explicit downCast() method to cast the result of setBufferData() back to a ConcreteBufferBuilder. The resulting code would look like this:
    class AbstractBufferBuilder {
    public:
        virtual ~AbstractBufferBuilder() = default;
    
        AbstractBufferBuilder& setBufferData() {
            return *this;
        }
    
        template <typename T, std::enable_if_t<std::is_base_of_v<AbstractBufferBuilder, T>, bool> = true>
        T& downCast() {
            assert(dynamic_cast<T*>(this));
            return *static_cast<T*>(this);
        }
    };
    
    class ConcreteBufferBuilder : public AbstractBufferBuilder
    {
    public:
        ConcreteBufferBuilder& addAttribute() {
            return *this;
        }
    
        std::unique_ptr<Buffer> build() {
            return std::make_unique<Buffer>();
        }
    };
    
    int main() {
        auto buffer = ConcreteBufferBuilder {}
            .setBufferData()
            .downCast<ConcreteBufferBuilder>()
            .addAttribute()
            .build();
    }
    

    Both have no runtime overhead (in Release mode anyway), but option 2. changes a bit the way the client should use the buffer creation API, and may potentially crash at run-time if an invalid down cast is performed (although both the std::enable_if and the assertion should catch common issues).

    Edit: One can also reconsider the use of the builder pattern here and instead encode what you desire as data types. As an example:

    struct BufferData {};
    struct Attribute {};
    
    struct BufferOptions {
        virtual ~BufferOptions() = default;
        virtual std::span<const Attribute> getAttributes() const = 0; // May be empty
        virtual std::optional<BufferData> getBufferData() const = 0;  // May be std::nullopt
    };
    
    struct ConcreteBufferOptions : BufferOptions {
        std::span<const Attribute> getAttributes() const { return attributes; }
        std::optional<BufferData> getBufferData() const { return data; }
    
        BufferData data;
        std::vector<Attribute> attributes;
    };
    
    std::unique_ptr<Buffer> createBuffer(const BufferOptions& options) {
        // TODO: Use the options to create the buffer
        return std::make_unique<Buffer>();
    }