14

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.

sivabudh
  • 31,807
  • 63
  • 162
  • 228

9 Answers9

22

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.

Community
  • 1
  • 1
Todd Gamblin
  • 58,354
  • 15
  • 89
  • 96
  • You mentioned destructor; if I have a bunch of objects inside a std::list, will they be destroyed once the std::list is destroyed? Or do I have to empty the list manually by myself before cleaning. – sivabudh Mar 25 '09 at 20:51
12

why you need extend vector in this way?

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

std::min_element, std::max_element

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

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.

bayda
  • 13,365
  • 8
  • 39
  • 48
  • But doesnt the work on an item at a time? My particular case needs to work on the data members. i.e. ::a, ::b, or ::c...? – sivabudh Mar 24 '09 at 23:00
  • @ShaChris23: most of algorithms could accept functors which will get your members – bayda Mar 24 '09 at 23:03
  • @ShaChris23: fixed avg sample – bayda Mar 24 '09 at 23:05
  • I'm not familiar boost::bind...could you please describe what _1 and _2 are or mean? reference: boost::bind( &accumulate_func, v.front(), _1, _2 ) ); – sivabudh Mar 24 '09 at 23:19
  • boost::bind - function which creates functor object from another function or functor. Binder allow you define parameters for function from which new functor will created _1, _2 - input parameters in newfunctor. – bayda Mar 24 '09 at 23:27
  • http://www.boost.org/doc/libs/1_38_0/libs/bind/bind.html - standard boost::bind documentation, if my explanation a little complicated. – bayda Mar 24 '09 at 23:27
11

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: or for c++11 and later, it can be as simple as:

const int result = std::max_element(v.begin(), v.end(), [](const Item &item1, const Item &item2) {
    return item1.a < item2.a;
}).a;

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<Item> or whatever. It is simply more versitile and helps code reuseablity.

Plus the functions in <algorithm> do 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 std::string).

Community
  • 1
  • 1
Evan Teran
  • 87,561
  • 32
  • 179
  • 238
  • Indeed: writing functions for iterators is a _way_ more powerful concept than inheriting a bag of member functions. +1 you! – xtofl Mar 25 '09 at 11:51
5

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.

Bruce Ikin
  • 885
  • 4
  • 6
3

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.

Paul
  • 13,042
  • 3
  • 41
  • 59
  • 1
    sure, it's "ok". But there is no point since you can only rely on the public interface of std::vector anyway. So you may as well just operate on that without the hassle of inheritance. – Evan Teran Mar 24 '09 at 23:06
2

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

1

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.

Peter
  • 7,216
  • 2
  • 34
  • 46
  • I agree with what you are saying. However, dont you think it would be nice if the MaxA() function is part of your container class? I mean..the MaxA is expected to work with vector container afterall. – sivabudh Mar 24 '09 at 23:02
  • It would be nice, yes, but it's a bad idea b/c STL isn't designed for inheritance. See my answer. – Todd Gamblin Mar 24 '09 at 23:12
1

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.

AHelps
  • 1,782
  • 11
  • 17
0

Just write a class that has a vector as a private member and write public interface functions that act on that vector.

edit: I saw that AHelps already gave the same answer so bump...

Eyjo
  • 1