-1

Assume the following class structure:

class Base;

class A : public Base;

class B : public Base;

class WrapperBase;

class AWrapper : public WrapperBase;

class BWrapper : public WrapperBase;

I would like to create some function like below

std::unique_ptr<WrapperBase> create(const Base* v);

That creates the appropriate "wrapper" class based upon the type of the "base" class passed in. To me, this means some sort of factory. However, I am not sure of the "best" approach to implement the factory. Here are some of the options I thought of:

Option A

Use a sequence of dynamic casts until the correct type is found:

std::unique_ptr<WrapperBase> create(const Base* v) {
    A* a = dynamic_cast<const A*>(v);
    if (a) return std::make_unique<AWrapper>(a);

    B* b = dynamic_cast<const B*>(v);
    if (b) return std::make_unique<BWrapper>(b);

    return std::unique_ptr{};
}

The downside of this method is obviously performance as a) dynamic casts are slow and b) many dynamic casts have to potentially be performed and c) this requires RTTI to be enabled.

Option B

Use some for of "id" field for lookup.

Assume the Base class has the following virtual method

enum class BaseID {
    A,
    B
};

virtual BaseID id() const = 0;

That could then be used as follows in the factory:

std::unique_ptr<WrapperBase> create(const Base* v) {
    switch(v->id()) {
    case BaseID::A:
        return std::make_unique<AWrapper>(static_cast<const A*>(v));
    case BaseID::B:
        return std::make_unique<BWrapper>(static_cast<const B*>(v));
    }
}

This is much faster, however I know that anytime a switch statement over object types appears, this smells of a bad design.

Option C

Use the visitor pattern. This would required the following modification:

class BaseVisitor;
class A;
class B;

class Base {
public:
    virtual void accept(BaseVisitor& visitor) const = 0;
};

class BaseVisitor {
public:
   virtual void visit(const A& v) = 0;
   virtual void visit(const B& v) = 0;
};

class A : public Base {
public:
    void accept(BaseVisitor& visitor) const override { visitor.visit(*this); }
};

//Same for B

class WrapperFactory : public BaseVisitor {
public:
    std::unique_ptr<WrapperBase> create(const Base& v) {
        v.accept(*this);
        return std::move(_tmp);
    }

    void visit(const A& v) override {
        _tmp = std::make_unique<AWrapper>(v);
    }
    void visit(const B& v) override {
        _tmp = std::make_unique<AWrapper>(v);
    }

private:
    std::unique_ptr<WrapperBase> _tmp;
};

This seems a bit "clunky" to me, but perhaps it is a good solution.

My question is, is there a different solution I am not considering, which would be:

  1. Easy to maintain
  2. Easy to read
  3. Safe
  4. More "OOP friendly"
2
  • I guess you forgot to mention a requirement: you probably don't want A to depend on WrapperA or B to depend on WrapperB, since it would result in a cyclic dependency, right?
    – Doc Brown
    Feb 10 at 15:44
  • @DocBrown That is correct. Feb 10 at 18:09

1 Answer 1

1

The most direct solution would be to replace the Wrapper* create(Base*) function with a virtual Wrapper* Base::wrap() method. Subclasses can override that method to select a suitable wrapper. This is simple and therefore elegant.

The visitor pattern achieves the same purpose, but without introducing a Base→Wrapper dependency. Whether this matters depends on the context of your code. Introducing an enum more or less saves the same purpose as the visitor, but is a more C-like approach to this problem.

Dynamic casts can be problematic because you can't select an appropriate wrapper class for every instance of your class hierarchy – your example has to fall back to a nullptr. Using a visitor is a more type-safe implementation of this approach, but note that both variants effectively use dynamic casts – one via the method call, the other more explicit. Using a dynamic_cast is not inherently slow, only your argument that multiple attempted casts would have to be chained is reasonable (and then only if this is a performance-sensitive part of your code and if there are very large class hierarchies). That dynamic casts require RTTI should not be a problem since that's an inherent feature of object-oriented C++. Disabling RTTI it is not worth the effort outside of memory-constrained embedded systems where you'd also disable exceptions and other core language features.

I would:

  • Choose a virtual wrap() method if this is purely internal code and a Base→Wrapper dependency is reasonable in this context. In particular, this might be appropriate for internal code where you can always refactor the entire Base class hierarchy.

  • Choose the visitor pattern if this is externally consumed code so that you can't always freely refactor, or if you want to avoid a direct Base→Wrapper dependency.

  • Choose the dynamic cast approach if you can't change the Base hierarchy at all (e.g. due to ABI guarantees that make it impossible to add a virtual method) or if you only need to wrap a few specific Base subclasses.

  • Not use the enum approach since it's less typesafe than all alternatives.

2
  • The Base->Wrapper dependency would imply dependencies from A to WrapperA and and B to WrapperB, which is precisely what the OP excluded, see my comment below the question.
    – Doc Brown
    Feb 10 at 19:45
  • @DocBrown I don't think such a dependency would be a problem for internal code, even if it's a circular dependency. I know, I know, that is “evil”, “not clean”, and “wrong”. But when possible just adding a method to a class is going to be much simpler and arguably more maintainable than reading up on design patterns. Of course it would be a problem when the dependencies cross module boundaries, and my answer does discuss the alternatives.
    – amon
    Feb 10 at 20:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.