Hi all,

I would like to pose a design pattern to discussion that is achieved in two different ways. Imagine the situation where you have an interface "IFoo" (aka virtual) and implementations "FooA", "FooB", ... that inherit and implement "IFoo". Now you want the user (of the library you are creating, thus user of the source code) to use the "FooA", "FooB", ... but you dont want the user to have to mess with all the different "FooX"s. Instead you want to create a "wrapper" / "top" "Foo" which shall be used by the users and that "Foo" shall handle one of the underlying implementations "FooA", "FooB", ... that is chosen upon creation of "Foo".

Below two ways to achieve this. The first example uses a shared pointer to handle an instance of on one the special "FooA", "FooB", ... and implements all the same methods as "FooA", "FooB" .. since it also inherits "IFoo". But its' methods are just calling the underlying "FooX"'s methods.
The second example uses a templated "Foo" that inherits from one of the "FooA", "FooB" and thus provides intrinsically all the methods of the underlying "FooX".
Both compile using "g++ -std=c++0x -Wall example.cpp"

I would like to hear any thoughts on both ways to reach this. Thanks!

example1:

#include <iostream>
#include <memory>

using namespace std;

/**
 * The interface Foo
 */
class IFoo {
public:

    virtual void foo() const = 0;

};

/**
 * First specific implementation
 */
class FooA : public IFoo {
public:

    void foo() const {
        cout << "Implementation specific foo A!!" << endl;
    }

};

/**
 * Second specific implementation
 */
class FooB : public IFoo {
public:

    void foo() const {
        cout << "Implementation specific foo B!!" << endl;
    }

};


/**
 * The foo class that can be FooA, FooB, ...
 */
class Foo : public IFoo {

    std::shared_ptr<IFoo> impl_ptr;

    Foo(shared_ptr<IFoo> impl)
        : impl_ptr(impl)
    {}

public:

    static Foo create(int impl) {
        if (impl == 1) {
            shared_ptr<IFoo> temp(new FooA());
            return Foo(temp);
        } else {
            shared_ptr<IFoo> temp(new FooB());
            return Foo(temp);
        }
    }

    void foo() const {
        impl_ptr->foo();
    }

};

int main(int argc, char **args) {

    Foo foo1 = Foo::create(1);
    foo1.foo();
    Foo foo2 = Foo::create(2);
    foo2.foo();

    return 0;
}

example2:

#include <iostream>
#include <memory>

using namespace std;

/**
 * The interface Foo
 */
class IFoo {
public:

    virtual void foo() const = 0;

};

/**
 * First specific implementation
 */
class FooA : public IFoo {
public:

    void foo() const {
        cout << "Implementation specific foo A!!" << endl;
    }

};

/**
 * Second specific implementation
 */
class FooB : public IFoo {
public:

    void foo() const {
        cout << "Implementation specific foo B!!" << endl;
    }

};

enum TFOO
{ FOO_A, FOO_B };

/**
 * The foo class that can be FooA, FooB, ...
 */
// DUMMY default class without content
template<TFOO T>
class Foo 
{};
// REAL class with content if FooA case
template<>
class Foo<FOO_A> : public FooA
{};
// REAL class with content if FooB case
template<>
class Foo<FOO_B> : public FooB
{};

int main(int argc, char **args) {
    Foo<FOO_A> foo_a = Foo<FOO_A>();
    foo_a.foo();
    Foo<FOO_B> foo_b = Foo<FOO_B>();
    foo_b.foo();

    return 0;
}

Recommended Answers

All 8 Replies

Not sure if it's just me or not but I think the first example might be leaking? No virtual destructor so when std::shared_ptr calls delete on the IFoo, doesn't the destructor of the actual object NOT get called?

I'm actually not sure how I'd debug it to tell..

my understanding is it does not.
Since the base class "IFoo" of "FooA" or "FooB" is not actually virtually inherited it does have an implicit destructor and so will thus "FooA" and "FooB". It would have a deleted destructor, and thus "FooA" and "FooB" would have deleted destructors, if it was virtually inherited: "The implicitly-declared or defaulted destructor for class T is defined as deleted(since C++11) if T has direct or virtual base class that cannot be destructed (has deleted or inaccessible destructors)" (C++ reference)

triumphost is correct about the destruction. The destructors of the FooA or FooB classes will never be called in your code. Instead, only the (default) destructor of the IFoo class will be called. You need to create a virtual destructor in the IFoo class:

class IFoo {
public:
    virtual void foo() const = 0;
    virtual ~IFoo() { };
};

The thing is, the problem is not about the destructors being "deleted" in the derived classes, but rather that the derived-class destructors are unreachable from the destruction (through delete in this case) through a base-class pointer. If the base-class destructor is not virtual, then when you destroy a base-class object, it calls the base-class destructor (default or not) directly. In other words, it won't call the derived-class destructor, as it should. You need the base-class destructor to be virtual for the destruction to be dispatched to the correct "most-derived" destructor.

As for your general problem, I'm not exactly sure what you hope to achieve. From what I understand, you want to encapsulate the existence of the FooX classes. I would not recommend the template version, just because you don't really gain anything from doing this, all you are doing is changing the name of the classes that the user sees, i.e., instead of seeing FooA, the uses sees Foo<FOO_A>, which doesn't seem particularly helpful as it does not provide encapsulation ("hide what it really is") nor polymorphic construction ("construct whatever is appropriate").

I would say, you can either go for a simple abstract factory pattern, or a wrapped abstract factory pattern.

With a simple abstract factory, you would just do this:

In foo.h:

#ifndef MY_LIB_FOO_H
#define MY_LIB_FOO_H

#include <memory>

/**
 * The interface Foo
 */
class IFoo {
public:
    virtual void foo() const = 0;
    virtual ~IFoo() { };
};

std::unique_ptr<IFoo> CreateFoo(int impl);

#endif

And, in foo.cpp:

#include "foo.h"

/**
 * First specific implementation
 */
class FooA : public IFoo {
public:
    void foo() const {
        cout << "Implementation specific foo A!!" << endl;
    }
};

/**
 * Second specific implementation
 */
class FooB : public IFoo {
public:
    void foo() const {
        cout << "Implementation specific foo B!!" << endl;
    }
};

std::unique_ptr<IFoo> CreateFoo(int impl) {
    if (impl == 1) {
        return std::unique_ptr<IFoo>(new FooA());
    } else {
        return std::unique_ptr<IFoo>(new FooB());
    };
};

And then, in the main:

#include "foo.h"

int main() {
    auto foo1 = CreateFoo(1);
    foo1->foo();
    auto foo2 = CreateFoo(2);
    foo2->foo();
    return 0;
}

And that's it. There is rarely much purpose for an additional layer of wrapping, just using a std::unique_ptr<IFoo> will do the trick (which you can also convert to a std::shared_ptr<IFoo> if you need shared ownership). In this technique, we get both the encapsulation of what kind of derived class is being used, and the polymorphic effect of "creating whatever is appropriate".

If you want to add the wrapping of that polymorphic pointer into a class like Foo, then you can just replace the factory function with a class that wraps a pointer. Again, the best pointer to use is either a raw pointer or a std::unique_ptr. But std::unique_ptr is probably the easiest (and has no overhead compared to a raw pointer) because you won't have to worry about move, copy and destruction, it will be automatically destroyed, movable, and non-copyable. Whether the inheritance from IFoo is needed for the Foo class is a matter of how you expect the user to use Foo objects, but let's say that this is something you want, then here is how it would look like:

In foo.h:

#ifndef MY_LIB_FOO_H
#define MY_LIB_FOO_H

#include <memory>

/**
 * The interface Foo
 */
class IFoo {
public:
    virtual void foo() const = 0;
    virtual ~IFoo() { };
};

class Foo : public IFoo {
  private:
    std::unique_ptr<IFoo> p_impl;
  public:
    void foo() const {
      p_impl->foo();
    };

    Foo(int impl = 1);
};

#endif

And, in foo.cpp:

#include "foo.h"

/**
 * First specific implementation
 */
class FooA : public IFoo {
public:
    void foo() const {
        cout << "Implementation specific foo A!!" << endl;
    }
};

/**
 * Second specific implementation
 */
class FooB : public IFoo {
public:
    void foo() const {
        cout << "Implementation specific foo B!!" << endl;
    }
};

Foo::Foo(int impl) : p_impl() {
    if (impl == 1) {
        p_impl = std::unique_ptr<IFoo>(new FooA());
    } else {
        p_impl = std::unique_ptr<IFoo>(new FooB());
    };
};

And then, in the main:

#include "foo.h"

int main() {
    Foo foo1(1);
    foo1.foo();
    Foo foo2(2);
    foo2.foo();
    return 0;
}

But again, in this trivial case, all you really gain is that the polymorphic pointer (p_impl) looks like an object (of class Foo). This will only be advantageous if you intend to add functionality or something to the Foo class, but note that this is also achievable, in general, with protected virtual functions (and non-virtual interface functions in IFoo).

@mike_2000_17 http://stackoverflow.com/questions/4195691/to-use-shared-ptr-is-it-safe and http://stackoverflow.com/questions/3899790/shared-ptr-magic
and even http://stackoverflow.com/questions/3899688/default-virtual-dtor/3899726#3899726

Apparently shared_ptr is implemented in such a way that the virtual destructor isn't needed.. It is apparently the only container implemented in such a way.

Live Test case: http://ideone.com/D2Lqoa

I was pretty shocked that was even possible.. Does not work for anything other than a shared_ptr. @OP, I am fairly certain you would be leaking if it wasn't for the accidental choice of using shared_ptr.

I guess that might make it safer to derive from stl containers such as std::string

It's actually the first time I ever heard of that or even saw something like that.

That is a nice little find. I didn't know that shared-ptr had this behavior built-in. But now that I think about, it makes perfect sense. One of the required features of shared-ptr is to be able to share ownership of one object through different shared-pointers which could be different kinds of pointers (to different base classes or data-members), so, there needs to be mechanisms included within the shared-ptr implementation to keep track of the original raw-pointer type (and value) that the very first shared-pointer was created with, and only delete that pointer. This is how you get the correct destructor called, as long as you create the shared-pointer with a raw pointer of the most derived type (which is usually the case).

But still, that doesn't mean that you shouldn't make the destructors virtual in the base class. It's an oddity, i.e., an artifact of the implementation of shared-ptr. And in any case, the existence of one way to create and use derived-class objects safely (even without a virtual destructor), does not make it "safe", because there were already plenty of ways to do this, it's the presence of all the very easy ways to have broken code as a result of a non-virtual destructor that make it unsafe, and those have not gone away.

I was pretty shocked that was even possible..

Oh.. this is easy. It's a simple type-erasure technique. It certainly adds overhead to the implementation, and this is why shared-pointers are expensive compared to raw pointers or unique-pointers, but it's not challenging to achieve. Other examples of type-erasure are std::function or boost::any. You can also see some interesting related techniques in one of my latest tutorials.

I guess that might make it safer to derive from stl containers such as std::string

The fact that the STL containers don't have a virtual destructor is only one of many reasons why you should not derive classes from them. This is a really really bad idea, and asinine design practices. There are classes that are designed to be derived and there are classes that are not, period. The only standard class that I know of that is designed to be derived (by a user) is the std::streambuf class (and that's not particularly easy, but it's what you would do if you wanted to create an io-stream class that dealt with something other than a file or a string).

Thank you for your hints and considerations! Very interesting stuff!

While I did not know in detail how shared_ptr was implemented, I was aware that it stores a deleter and does not rely upon a "dtor chain", so not a completely accidental choice. But I recon its safer to have the explicit virtual dtor. However I was not aware that unique_ptr does not support this!

As for the design question, thank you for the examples! Now regarding the classic factory pattern, thats not for us for mostly 2 reasons:
1.) It does still show all the FooX classes to the user, which also makes the doc more complicated. We want him to see just one Foo class.
2.) We want to add higher level, common functions to Foo - and not to IFoo! Again the reason being the doc: we dont want the user to see IFoo.
Of course one could write the doc independently of the code for an imaginary "Foo". But we prefer having the doc tied to the code so that methods get documented for classes they reside actually in.

Regarding the wrapped factory pattern: this is basically equivalent to the original example with the only difference being that the constructor creates the stored object and not a separate "create". However, in my example as you can see the constructor instead takes a pointer for shared_ptr and thus allows fun tricks on copy operations, you dont have to copy the whole underlying "FooX" but just take the pointer to it. This seemed better to us since the "FooX" are going to be BIG.

Regarding the template pattern: I do not completely agree with your summary. The user does see "Foo<FOO_A>" instead of "FooA", correct. But he sees only "Foo<FOO_A>" and not all the "FooA", "FooB", ... so you do again have the wrapper effect and the one "Foo" to take higher level functions and associated documentation.
Also I do not see why it would not provide polymorphism, I think that's exactly what it does just not the "constructor way". Depending on which FooX it inherits it will automatically have different structure and this is the actual big difference to the other patterns: it does not have to inherit IFoo directly but does so from "FooX" and the whole inheritace chain is much clearer.
The only thing you loose (I would have said) is the elegant copy operation trick (switch pointer).

But please feel free to critisize all I said, that's why Im here! :) Thanks for your help.

P.S.: I recon that you could do the same copy tricks with your wrapped factory pattern (if constructor is creating) by offering a separate switch function but in the end both seems really the same to me - but I may be wrong. :)

1.) It does still show all the FooX classes to the user, which also makes the doc more complicated.

That's not true, in the factory pattern, the user only sees the IFoo class. The FooX classes are encapsulated in the cpp file where the factory function is defined, and are thus not in any header and don't need to be distributed with your library, and are not visible to the user at all. The only class that the user has to interact with is IFoo.

2.) We want to add higher level, common functions to Foo - and not to IFoo!

Like I said, if this is the case, use the wrapped pattern.

Again the reason being the doc: we dont want the user to see IFoo.

If you look at the wrapped pattern, you will see that there is no particular need for IFoo to be exposed at all. A simple forward-declaration will do the trick:

In foo.h:

#ifndef MY_LIB_FOO_H
#define MY_LIB_FOO_H

#include <memory>

class IFoo; // <- forward declaration.

class Foo {
  private:
    std::unique_ptr<IFoo> p_impl;
  public:
    void foo() const;

    Foo(int impl = 1);
};

#endif

In foo.cpp:

#include "foo.h"

class IFoo {
public:
    virtual void foo() const = 0;
    virtual ~IFoo() { };
};

class FooA : public IFoo {
public:
    void foo() const {
        cout << "Implementation specific foo A!!" << endl;
    }
};

class FooB : public IFoo {
public:
    void foo() const {
        cout << "Implementation specific foo B!!" << endl;
    }
};


void Foo::foo() const {
    p_impl->foo();
};

Foo::Foo(int impl) : p_impl() {
    if (impl == 1) {
        p_impl = std::unique_ptr<IFoo>(new FooA());
    } else {
        p_impl = std::unique_ptr<IFoo>(new FooB());
    };
};

With this, the IFoo class is entirely hidden in the cpp file (except for a forward-declaration, which is nothing). The user will be completely oblivious to the existence of either IFoo or any of its derived classes, unless you give them the source code of your library's binaries (or that the even bother to look).

However, in my example as you can see the constructor instead takes a pointer for shared_ptr and thus allows fun tricks on copy operations, you dont have to copy the whole underlying "FooX" but just take the pointer to it.

That's a good "trick" for sure, but it is no longer a copy if you actually share the same under-the-hood object between the two Foo objects after the copy. You have to be careful about this kind of thing because breaking language semantic rules, like "a copy duplicates the entire state of an object", is going to be far more aggregious to your users than some superfluous bits of docs. This trick would only be semantically correct if you use either a "copy-on-write" scheme, or if all your FooX classes are stateless classes. In the former case, you have quite a few more things to handle (and it's much more case-specific). In the latter case, you should probably be using a singleton feature of some kind, but it depends on the specifics too (do FooX classes contain immutable data members, or do they contain no data members at all? (these are the only two choices for a "stateless" class)).

Also remember that in standard semantics, the way to pass along an object while avoiding copying it is to pass it by reference. This is something all users should be comfortable with and used to doing, so, there isn't necessarily a big need to have this kind of trick built-in to the classes.

And remember, C++11 has move-semantics, which give you a natural way to "pass over the internal pointer" instead of doing a deep copy, as long as the source object can be "zombiefied" (left empty). This has eliminated much of the need for these kinds of tricks, because it gives you a choice between a full copy and a cheap move, and in most cases, the best choice is made transparently.

IMHO, I think this trick has more down-sides than up-sides. Even a copy-on-write is rarely worth the trouble. A lot of the programming flow of C++ depends on direct value-semantics, and if you start messing with that, there are lots of down-sides. And, like I said, doing this might not be as useful as you think. Personally, this is one of the "marks of the beast" that I use to detect poorly written C++ libraries.

But he sees only "Foo<FOO_A>" and not all the "FooA", "FooB", ... so you do again have the wrapper effect and the one "Foo" to take higher level functions and associated documentation.

What do you mean by "see"? Because there are a few different meanings to that word in this context:

1) If you mean "see" to mean that the user will have to invoke that class directly in their code, then it is true that in the template version, the user doesn't "see" the FooX classes.

2) If you mean "see" to mean that the user will have to look up some documentation related to what the class does, what it inherits from, or what function it has, then the template version definitely makes the FooX classes "visible" to the user, and you will definitely have to document them.

3) If you mean "see" to mean that the user will be able to look into the library headers and find the definition of the class, then the template version is again very obviously making the FooX classes "visible" to the user. This is by requirement of the templates, as they need to have FooX classes visible in included library headers in order to be instantiationable for the user, and if the compiler can "see" it, the user can "see" it. This is the definition of "see" that I usually use. And this is also the definition used when people talk about "encapsulation" (i.e., making things not "visible" to the user).

And then, you have to understand something about templates. Each instantiation is a distinct class, and that's all they are. So, there is no practical difference between Foo<FOO_A> and FooA, except that the template version is more wordy and ugly. That's the point. It gives you an illusion that they are a single class Foo, but it's actually a class template, and from it you have several distinct classes Foo<FOO_A>, Foo<FOO_B>, ..., and you will need distinct documentation for each, and you will need to expose to the user the existence of FooX classes in the process (and document them).

Also I do not see why it would not provide polymorphism, I think that's exactly what it does just not the "constructor way".

I didn't say that it would not provide "polymorphism", I said it would not provide "polymorphic construction". That's not at all the same thing. Polymorphic construction means that what the user sees is "I construct a Foo object with X,Y,Z parameters" while the implementation actually does "I'm given X,Y,Z parameters and choose the appropriate and compatible object type to construct and return under the disguise of a Foo object". In other words, the true nature of the object being created is hidden from the caller. This is certainly not the case for the template version because the caller has to know which distinct class between Foo<FOO_A>, Foo<FOO_B>, ..., is going to be created. The construction is not polymorphic, and that was my point.

In terms of having polymorphic functions, i.e., dynamic dispatching, then all the version have this, they have to have this, it is what you requested to begin with. And it is also a trivial effect to achieve, so it is not really interesting to discuss.

Depending on which FooX it inherits it will automatically have different structure and this is the actual big difference to the other patterns: it does not have to inherit IFoo directly but does so from "FooX" and the whole inheritace chain is much clearer.

The inheritance chain is much "clearer", but it is also entirely exposed. It was my understanding that you do not wish to expose and document the details and existence of the FooX classes (or even IFoo). This solution seems like a very very poor choice if that is part of your objectives. In a library, you cannot have classes (template instantiations or otherwise) that inherit from classes that you leave undocumented, or at least, it is very bad practice to do so. This solution really achieves no encapsulation whatsoever, and if you are OK with that, then I really don't understand what you want, because it seems to contradict everything else you said.

I recon that you could do the same copy tricks with your wrapped factory pattern (if constructor is creating) by offering a separate switch function but in the end both seems really the same to me - but I may be wrong. :)

Are you talking about a "swap" function? Well, with C++11, the standard swap function relies on move operations, which are generated by default, and in the case of using unique_ptr, the move operations are just a simple pointer copy, which is what I assume is your "switch pointer trick". So, this optimization comes for free in the wrapped pattern, you don't even have to do anything more.

But please feel free to critisize all I said, that's why Im here! :) Thanks for your help.

I hope I didn't disappoint. ;)

Thanks for the lengthy considerations and good thoughts, clears some things! I agree on your criticism regarding the non-standard nature of the semantics that arise from the copy tricks, it kind of breaks the C++ way - aside that it is / would be still quite nice I think.

The considerations regarding the factory patterns: I think we agree the wrapped one would be our way. Now how that compares to the templating, good question.
I see your point that the user "sees" the underlying classes with the templating version, except if you dont document them which was kind of my thought - but again I guess you are right this is not elegant. I do understand very well what templates are, I played a lot with them. I have to say I still do not entirely agree that there is little or no difference between "FooA" and "Foo<FOO_A>". As a developer, in the first case I have to look at a class "FooA", at a "FooB" and so on, with all their unique code. In the 2nd case I look at a "Foo" which extends the "FooX", and if Im curious I go deeper looking at "IFoo" or one specific "FooX". I perceive this as being more easy and structured. But I guess here it really comes down to personal preferences and programming styles.

However let me thank you again for the deep insights and very nice considerations, I appreciate it very much!

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.