Advice on a better way to extend C++ STL container with user-defined methods

Question

I inherited from C++ STL container and add my own methods to it. The rationale was such that to the clients, it will look act a regular list, yet has application-specific methods they can readily be called.

This works fine, but I have read numerous posts about not inheriting from STL. Can someone provide a concrete advice of how I might write the code below in a better way?

class Item
{
  int a;
  int b;
  int c;

  int SpecialB()
  {
    return a * b + c;
  }
};

class ItemList : public std::vector<Item>
{
  int MaxA()
  {
    if( this->empty() )
      throw;

    int maxA = (*this)[0].a;

    for( int idx = 1; idx < this->size(); idx++ )
    {
      if( (*this)[idx].a > maxA )
      {
        maxA = (*this)[idx].a;
      }
    }
    return maxA;
  }

  int SpecialB()
  {
    if( this->empty() )
      throw;

    int specialB = (*this)[0].SpecialB();

    for( int idx = 1; idx < this->size(); idx++ )
    {
      if( (*this)[idx].SpecialB() < specialB )
      {
        specialB -= (*this)[idx].c;
      }
    }
    return specialB;
  }

  int AvgC()
  {
    if( this->empty() )
      throw;

    int cSum = 0;
    for( int idx = 0; idx < this->size(); idx++ )
    {
      cSum += (*this)[idx].c;
    }

    return cSum / this->size(); // average
  }
};

EDIT: Thanks for a bunch of thoughtful answers. I will create helper functions instead and from now on will never inherit from STL containers.

Answer 1

why you need extend vector in this way?

use standard <algorithm> with your functors.
e.g.

std::min_element, std::max_element - http://www.sgi.com/tech/stl/min_element.html

int max_a = std::max_element
  ( 
   v.begin(), 
   v.end(), 
   boost::bind( 
    std::less< int >(),
    bind( &Item::a, _1 ), 
    bind( &Item::a, _2 ) 
   )
  )->a;

std::accumulate - for calculate avarage http://www.sgi.com/tech/stl/accumulate.html

const double avg_c = std::accumulate( v.begin(), v.end(), double( 0 ), boost::bind( Item::c, _1 ) ) / v.size(); // ofcourse check size before divide

your ItemList::SpecialB() could be rewrited as:

int accumulate_func( int start_from, int result, const Item& item )
{
   if ( item.SpecialB() < start_from )
   {
       result -= item.SpecialB();
   }
   return result;
}

if ( v.empty() )
{
    throw sometghing( "empty vector" );
}
const int result = std::accumulate( v.begin(), v.end(), v.front(), boost::bind( &accumulate_func, v.front(), _1, _2 ) );

BTW: if you don't need access to members, you don't need inheritance.

Answer 2

I can't see why you need to extend vector with those methods. You could just write them as standalone functions, eg:

int MaxA(const std::vector<Item>& vec) {
    if(vec.empty()) {
        throw;
    }

    int maxA = vec[0].a;
    for(std::vector<Item>::const_iterator i = vec.begin(); i != vec.end(); ++i) {
        if(i->a > maxA) {
            maxA = i->a;
        }
    }
    return maxA;
}

Or there's std::max_element which would do much the same... minus the throwing of course.

Answer 3

Since you can only "extend" the vector by using its public interface, it is far more useful to write functions which operate on a vector instead of being part of a vector.

Heck, if you plan it well, make it work with iterators instead of indexes and it'll work with more than just std::vector (see <algorithm> for some very good examples).

For example, you could use a functor for the MaxA like this:

struct CmpA {
    bool operator()(const Item &item1, const Item &item2) { return item1.a < item2.a; }
}

const int result = std::max_element(v.begin(), v.end(), CmpA()).a;

your specialB could be just as simple with a functor and std::accumulate

EDIT: you've asked why it is better to do it this way:

if you use algorithms, templates and iterators, it'll work even if you decide to put the items in a std::list or whatever. It is simply more versitile and helps code reuseablity.

Plus std::algorithm does much of this for you so you can just use little 3 line adapter functors.

EDIT: In addition to this, tgamblin listed some very compelling reasons to not inherit from std::vector (and most other std containers, including string).

Answer 4

If the standard algorithms don't have what you need, just write free functions, preferably templated.

Answer 5

Warnings about not inheriting from STL containers appear because methods of STL containers are not virtual. So if you don't override methods and don't need polymorphic behaviour, but just extend the class — it's OK to inherit STL containers.

Answer 6

This is a bad idea.

There are a lot of reasons you shouldn't derive from STL classes, foremost of which is that they're not designed for it. Vector doesn't have a virtual destructor, so if you extend it, the superclass's destructor may not be called properly and you'll get memory leaks.

For more on this, see this answer on why not to derive from std::string. Many of the same points apply:

Constructor doesn’t work for class inherited from std::string

• No virtual destructor
• No protected functions (so you gain nothing by inheriting)
• Polymorphism won't work, and you'll get object slicing. std::vector is assignable, but if you add your own fields they won't get copied on assignment if you assign from a vector pointer or vector reference. This is because vector's operator= does not know about your fields.

For all of these reasons, you're better off making utility functions than extending when it comes to STL.

Answer 7

I prefer to use stl containers as an implementation detail rather than as part of my solution's interface. This way I can change containers (vector for deque or list) if the need arises without any of the calling code being affected. You may have to write a few call through functions but the encapsulation is worth the extra typing.

Answer 8

You're correct that you should not inherit from STL containers. Virtual functions would make them meaningfully larger -- the base size of vectors would jump from 12 to 16 bytes (in the implementation I'm using). Besides that, virtual functions are difficult to inline, which can slow down the code. If you find yourself making an array of a million mostly-empty vectors, the difference adds up pretty fast.

You can still get the effect you want by declaring the vector as a member variable in your ItemList and then thunking through any methods that you wish to expose.

class ItemList {
private:
  std::vector< Item > mItems;
public:
  typedef std::vector< Item >::size_type size_type;
  int MaxA();
  int SpecialB();
  Item &operator[]( size_type offset ) { return mItems[offset]; }
  size_type size() const { return mItems.size(); }
};

... and so forth. This is a fair amount of grunt work, but it will give you the effect you asked for.