8
\$\begingroup\$

Here is a small C++11 utility collection to store any object that satisfies a given interface. Only the basic concepts are provided, I did not try to create a full collection:

#include <memory>
#include <utility>
#include <vector>
#include <boost/iterator/indirect_iterator.hpp>

template <typename Interface, template <typename> class Adapter>
class poly_collection
{
public:
    using iterator = boost::indirect_iterator<typename std::vector<std::unique_ptr<Interface>>::iterator>;

    template<typename T, typename... Args>
    auto push(Args&&... args)
        -> void
    {
        auto ptr = new Adapter<T>{ std::forward<Args>(args)... };
        entities.emplace_back(ptr);
    }

    template<typename T>
    auto push(T&& other)
        -> void
    {
        auto ptr = new Adapter<T>{ std::forward<T>(other) };
        entities.emplace_back(ptr);
    }

    auto begin()
        -> iterator
    {
        return { std::begin(entities) };
    }

    auto end()
        -> iterator
    {
        return { std::end(entities) };
    }

private:
    std::vector<std::unique_ptr<Interface>> entities;
};

Here is an example of use. Imagine you have an abstract class named Shape. For the sake of simplicity, it will only have one abstract method, draw:

struct Shape
{
    virtual void draw() const = 0;

    virtual ~Shape() {};
};

Now, you have some nice classes, Rectangle and Circle, that you didn't write but that satisfy the Shape interface:

struct Circle
{
    Circle(int x, int y, int radius) {}

    void draw() const
    {
        std::cout << "Circle\n";
    }
};

struct Rectangle
{
    Rectangle(int x, int y, int height, int width) {}

    void draw() const
    {
        std::cout << "Rectangle\n";
    }
};

You would like to store Rectangle and Circle instances in a colleciton as if they inherited from a common base class. With our solution, you only have to write a new class: ShapeAdapter, and then feed it to the poly_collection:

template<typename T>
struct ShapeAdapter:
    Shape
{
    template<typename... Args>
    ShapeAdapter(Args&&... args):
        data(std::forward<Args>(args)...)
    {}

    virtual void draw() const override
    {
        data.draw();
    }

    T data;
};

int main()
{
    poly_collection<Shape, ShapeAdapter> shapes;

    shapes.push<Circle>(1, 2, 3);
    shapes.push<Rectangle>(4, 5, 6, 7);

    Circle c = { 8, 9, 10 };
    shapes.push(c);

    for (auto&& shape: shapes)
    {
        shape.draw();
    }
}

So, what do you think of the idea? Would that be an interesting utility? Is there any way I could improve such a collection appart from adding some functions to mimick the standard library's collections ones?

\$\endgroup\$
  • \$\begingroup\$ Is this a try in modeling what Sean Parent talked about here: channel9.msdn.com/Events/GoingNative/2013/… \$\endgroup\$ – Nobody Jul 16 '14 at 9:21
  • \$\begingroup\$ @Nobody No idea. Talks always bore me, so I never watch them. \$\endgroup\$ – Morwenn Jul 16 '14 at 9:22
  • \$\begingroup\$ @Nobody It has to do with the slides "value semantics and concept-based polymorphism" though. Maybe those are the slides used in the talk. \$\endgroup\$ – Morwenn Jul 16 '14 at 9:28
  • \$\begingroup\$ Parts of it are these slides (I believe). \$\endgroup\$ – Nobody Jul 16 '14 at 9:33
  • \$\begingroup\$ Why do you need the adapter? Seems like you could just put objects that implement the interface directly into storage. This feels like boost::ptr_vector. \$\endgroup\$ – Martin York Jul 16 '14 at 17:57
5
\$\begingroup\$

As usual with your code, I can only come up with very minor nitpicks.

With both push functions, there is a small chance of a memory leak. After the construction of a new Adapter<T> using:

auto ptr = new Adapter<T>{ std::forward<Args>(args)... };

If entities.emplace_back(ptr); fails (say if relocation is required, the vector needs to expand, and this fails), then ptr will leak. Instead of doing it this way, I'd suggest using make_unique and moving the unique_ptr into entities using push_back instead:

template<typename T, typename... Args>
auto push(Args&&... args)
    -> void
{
    auto ptr = std::make_unique<Adapter<T>>(std::forward<Args>(args)...);
    entities.push_back(std::move(ptr));
}
\$\endgroup\$
3
\$\begingroup\$

As suggested by LokiAstari I would also use a boost::ptr_* container which does the pointer wrapping for you.

Writing your own container(wrapper) puts the burden of forwarding some members onto you. For example your poly_collection does not define the usual pointer, reference, value_type (and some more) member types to identify as "real" container. Your usecase does not depend on this (and you might not intend to use them in any other way) but why should you worry about this, when you can do it another way.

Looking at your poly_collection it strikes me as odd that you have to store the adapter's type in it. Looking more closely this is only used in the push functions (which btw. should be called push_back or rather emplace_back to be more consistent with the naming in the stdlib).

Those functions could become free functions thereby leaving poly_collection without the dependency on the adaptor template:

namespace concept_based_polymorphism {

template <typename DerivedType, typename PtrContainer, typename... Args>
void push(PtrContainer &ptrContainer, Args &&... args) {
  using BaseClass =
      typename std::remove_pointer<typename PtrContainer::value_type>::type;
  std::auto_ptr<BaseClass> owningPointer(
      new DerivedType{std::forward<Args>(args)...});
  ptrContainer.push_back(owningPointer);
}

template <template <typename> class Adapter, typename T, typename PtrContainer>
void push(PtrContainer &ptrContainer, T &&other) {
  using BaseClass =
      typename std::remove_pointer<typename PtrContainer::value_type>::type;
  std::auto_ptr<BaseClass> ptr(new Adapter<T>{std::forward<T>(other)});
  ptrContainer.push_back(ptr);
}
}

These functions are designed to work on boost's pointer containers but you could overload them to work with other types of containers as well. The boost::ptr_ work with std::auto_ptr so the code is a bit uglier than it could be (one could write a make_auto_ptr helper analogous to make_unique to make it somewhat prettier).

Using them makes it mandatory to supply the Adapter type so you could provide overload/shortcuts per adapter:

template<typename DerivedType, typename PtrContainer, typename ... Args>
void push_shape(PtrContainer &ptrContainer, Args&&... args) {
    using concept_based_polymorphism::push;
    push<ShapeAdapter<DerivedType>>(ptrContainer, std::forward<Args>(args)...);
}

template<typename T, typename PtrContainer>
void push_shape(PtrContainer &ptrContainer, T&& other) {
    using concept_based_polymorphism::push;
    push<ShapeAdapter>(ptrContainer, other);
}

Given these shortcuts we can now do this:

int main()
{

    boost::ptr_vector<Shape> shapes;

    push_shape<Circle>(shapes, 1, 2, 3);
    push_shape<Rectangle>(shapes, 4, 5, 6, 7);

    Circle c = { 8, 9, 10 };
    push_shape(shapes, c);

    for (auto&& shape: shapes)
    {
        shape.draw();
    }
}

Which works like the code before. But with the added bonus that you can now easily switch the container and don't have to duplicate its interface.

If you want to retain the nicer push interface where the adaptor type is stored in the container, I would recommend a shallow wrapper that leaves the base container accessible and parametrizable, thus allowing for the "easier" push functions to be combined with the reduced code duplication of not having to forward the wrapped containers interface:

template <typename ContainerType, template <typename> class Adapter>
struct poly_container_adaptor {
    ContainerType container;
};

Now the pushes would work with poly_container_adaptor and "normal" code would just get passed the container.

\$\endgroup\$
  • \$\begingroup\$ The problem with the push_shape solution is that it forces to create a push_* method per adapter (the generic solution is to rewrite ShapeAdapter everytime we call push_shape, which isn't pretty) and uses a free function for what is usually a class method. I intended at some point to have a template parameter to switch containers as you propose, but since I didn't need that, I ended up not implementing it. \$\endgroup\$ – Morwenn Aug 18 '14 at 9:45
  • \$\begingroup\$ Your points about missing subtypes and functions names are good though. That said, I don't think that there could be any sane value_type here. \$\endgroup\$ – Morwenn Aug 18 '14 at 9:46
  • \$\begingroup\$ The boost::ptr_* containers define typedef pointer value_type maybe you could use that as an orientation what to do. The poly_container_adaptor would allow for a generic push member (without giving the adaptor). There is a problem with passing the adaptor as template template parameter: You can only use templates that fit this pattern. The push_* are more generic in that regard. Whether the functions should be members or not is more of a style preference. Both members and nonmembers can (&) and cannot (const &) change parameters. \$\endgroup\$ – Nobody Aug 18 '14 at 10:10
  • \$\begingroup\$ boost::ptr_* define value_type because it is clear from their design that they store pointers. I wanted a collection of instances derived from an "interface" without exposing the pointers anywhere, hence the lack of meaningful value_type (and I am no at ease with reference != value_type&). I don't think that the teamplte template is a problem since the collection already requires the user to write an adapter; it is not trying to use any existing class as the adapter. \$\endgroup\$ – Morwenn Aug 18 '14 at 12:35

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.