views:

422

answers:

7

Consider the following example code:

class Foo
{
};

class Bar : public Foo
{
};

class FooCollection
{
protected:
    vector<shared_ptr<Foo> > d_foos;
};

class BarCollection : public FooCollection
{
public:
    vector<shared_ptr<Bar> > &getBars()
    {
        // return d_foos won't do here...
    }
};

I have a problem like this in my current project. The client code uses BarCollection, which stores pointers to Bars in d_foos which is declared in FooCollection. I'd now like to expose the collection of pointers to Bars to the client code. I could just give the client code access to the vector of pointers to Foos and cast these to pointers to Bars in the client code, but this feels wrong since the client doesn't have to know about Foo's existence.

I could also define a get() member that retrieves objects from d_foos and casts them, but this feels quite clumsy. Preferably, I'd like to just return d_foos as a vector<shared_ptr<Bar> > &, but I cannot seem to do this.

It could also be that my design is just plain wrong. It seemed to most natural solution though, as Bar is a specialization of Foo and BarCollection is a specialization of FooCollection and they share functionality.

Could you suggest nice solutions to implement getBars in BarCollection or better design alternatives?

Edit:

Turns out my design was bad indeed. BarCollection is not a FooCollection, despite of requiring all of FooCollection's functionality. My current solution based on the answers below -- which is a lot cleaner -- is now:

class Foo
{
};

class Bar : public Foo
{
};

template<class T>
class Collection
{
    vector<shared_ptr<T> > d_items;
};

typedef Collection<Foo> FooCollection;

class BarCollection : public Collection<Bar>
{
    // Additional stuff here.
};

Thanks for all the excellent suggestions and examples!

+1  A: 

Can you not replace this with a Collection templatized on either Foo / Bar?, something like this

class Collection<T> {
protected:
    vector<shared_ptr<T> > d_foos;
};

typedef Collection<Foo> FooCollection;
typedef Collection<Bar> BarCollection;
Jeff Foster
Probably not if you want to provide a polymorphic method.
David Rodríguez - dribeas
+2  A: 
template<class T>
class MyContainer {
  vector<shared_ptr<T> > d_foos;
public:
  vector<shared_ptr<T> > & getVector();
};

class FooCollection : public MyContainer<Foo> {
};

class BarCollection : public MyContainer<Bar> {
};
Alexey Malistov
+5  A: 

The problem is you are trying to mix-and-match two different, pretty-much independent kinds of polymorphism in a way that won't work. The compile-time, type-safe polymorphism of templates won't allow you to substitute a base type for a derived type. C++'s template system makes no association between

class<Foo>

and

class<Bar>

One suggestion might be to create a Foo derived adapter that would cast down to the correct class:

 template <class derived, class base>
 class DowncastContainerAdapter
 {
 private:
     std::vector< boost::shared_ptr<base> >::iterator curr;
     std::vector< boost::shared_ptr<base> >::const_iterator end;
 public:
     DowncastContainerAdapter(/*setup curr & end iterators*/)
     {
         // assert derived actually is derived from base
     }

     boost::shared_ptr<derived> GetNext()
     {
         // increment iterator
         ++curr;
         return dynamic_cast<base>(*curr);
     }

     bool IsEnd()
     {
         return (curr == end);
     }
 };

Note this class will have the same problem as an iterator, operation on the vector may invalidate this class.

Another thought

You may not realize it, but it may be perfectly fine to just return a vector of Foo. The user of Bar already must have full knowledge of Foo, since by including Bar.h they must get Foo.h through Bar.h. The reason being that for Bar to inherit from Foo, it must have full knowledge of the class via Foo.h. I would suggest rather than using the solution above, if it's possible make Foo (or a super class of Foo) an interface class and pass around vectors of pointers to that interface class. This is a pretty commonly seen pattern and won't raise the eyebrows that this wonky solution I came up with might :). Then again you may have your reasons. Good luck either way.

Doug T.
@Another thought: well, Bar might introduce functionalities that you would really like to have, instead of casting yourself.
Matthieu M.
+3  A: 

I would suggest exposing iterators from your container classes, instead of the member container. That way it won't matter what the container type is.

Chris Bednarski
Matthieu M.
I meant custom iterators
Chris Bednarski
+2  A: 

The question is, why would you do that? If you give the user a collection of pointers to Bar, you assume, that there are only Bars in it, so internally storing the pointers in a collection to Foo makes no sense. If you store different subtypes of Foo in your collection of pointer to Foo, you cannot return it as a collection of pointers to Bar, since not all objects in there are Bars. In the first case, (you know that you have only bars) you should use a templated approach as suggested above. Otherwise, you have to rethink, what you really want.

Space_C0wb0y
+1  A: 

Do you have a special need to have BarCollection derived from FooCollection? Because generally a BarCollection is not a FooCollection, usually a lot of the things that can be done with a FooCollection should not be done with a BarCollection. For example:

BarCollection *bc = new BarCollection();
FooCollection *fc = bc; // They are derived from each other to be able to do this
fc->addFoo(Foo());      // Of course we can add a Foo to a FooCollection

Now we have added a Foo object to what is supposed to be a BarCollection. If the BarCollection tries to access this newly added element and expects it to be a Bar, all kinds of ugly things will happen.

So usually you want to avoid this and don't have your collection classes derived from each other. See also questions about casting containers of derived types for more answers on this topic...

sth
Good example of why my initial design was bad. +1!
TC
+1  A: 

First of all, let's talk about shared_ptr. Do you know about: boost::detail::dynamic_cast_tag ?

shared_ptr<Foo> fooPtr(new Bar());
shared_ptr<Bar> barPtr(fooPtr, boost::detail::dynamic_cast_tag());

This is a very handy way. Under the cover it just performs a dynamic_cast, nothing fancy there but an easier notation. The contract is the same that the classic one: if the object pointed to does not actually is a Bar (or derived from it), then you get a null pointer.

Back to your question: BAD CODE.

BarCollection is NOT a FooCollection, as mentioned you are therefore in trouble because you could introduce other elements in the vector of pointers that Bar ones.

I won't extend on that though, because this is beyond the question at hand, and I think that we (as those trying to answer) should restrain ourselves from that.

You cannot pass a reference, but you can pass a View.

Basically, a View is a new object that act as a Proxy to the old one. It's relatively easy using Boost.Iterators from example.

class VectorView
{
  typedef std::vector< std::shared_ptr<Foo> > base_type;

public:
  typedef Bar value_type;
  // all the cluttering

  class iterator: boost::iterator::iterator_adaptor<
    iterator,
    typename base_type::iterator,
    std::shared_ptr<Bar>
  >
  {
    typename iterator_adaptor::reference dereference() const
    {
      // If you have a heart weakness, you'd better stop here...
      return reinterpret_cast< std::shared_ptr<Bar> >(this->base_reference());
    }
  };

  // idem for const_iterator

  // On to the method forwarding
  iterator begin() { return iterator(m_reference.begin()); }

private:
  base_type& m_reference;
}; // class VectorView

The real problem here is of course the reference bit. Getting a NEW shared_ptr object is easy, and allow to perform a dynamic_cast as required. Getting a reference to the ORIGINAL shared_ptr but interpreted as the required type... is really not what I like to see in code.

Note:
There might be a way to do better than that using Boost.Fusion transform_view class, but I could not figure it out.

In particular, using transform_view I can get shared_ptr<Bar> but I can't get a shared_ptr<Bar>& when I dereference my iterator, which is annoying given that the only use of returning a reference to the underlying vector (and not a const_reference) is to actually modify the structure of the vector and the objects it contains.

Note 2:
Please consider refactoring. There have been excellent suggestions there.

Matthieu M.