views:

164

answers:

4

One of my C++ classes derives from std::vector so that it can act as a container that also perform custom actions on its content. Unfortunately, the compiler complains about the destructor not to be virtual, which I cannot change, since its in the standard library.

Am I doing the whole thing wrong (thou shall not derive from STL) or is there something I could do to keep the compiler happy ? (appart from stop using -Weffc++ :)

edit: the derived class do not touch the vector-manipulation algorithms, but merely add some information such as "element width/height" for a vector of images. As an example, you could think of

class PhotoAlbum: public std::vector<Photo> {
    String title;
    Date from_time, to_time;
    // accessors for title and dates
    void renderCover(Drawable &surface);
};

where you think of a photo album primarily as a collection of pictures with some meta-data (title and time) and album-specific features such as rendering a thumbnail of some Photo onto a surface to make the album cover. So imho, the photo album IS-A collection of Photo, more than it HAS-A such collection.

I fail to see any benefit I'd gain of having getPhotoVector() method in a PhotoAlbum that would have an extra "collection" field.

+15  A: 

Why not use composition? Simply make std::vector a member of your custom container, then implement the custom actions as member functions of said class that act upon the std::vector member. That way, you have complete control over it. Besides, you should prefer composition over inheritance if inheritance is not required.

In silico
or define the additional functionality as non-member functions.
jalf
+1  A: 

Maybe use composition instead of inheritance? You don't really say why you are extending vector so I don't know if this is an option.

Starkey
had good reason for not wanting composition when I wrote the code, but the point you (and others) raised is obviously valid. I'm going back to my code and study the option of doing the "right" way.
sylvainulg
+1  A: 

Am I doing the whole thing wrong

Possibly.

You say you have derived from vector in order to provide special functionality. The way this is typically done is by using the built-in algorithms in combination with your own functors to provide this special functionality.

Doing this instead of deriving from vector provides multiple benefits:

1) It helps to prevent scope creep in to vector. vector's job is to maintain a collection of objects. Not to perform algorithmic functions on those objects. By deriving from vector and adding your special functions, you make vector more complex because now you have given it another job to do.

2) It is more in line with the "STL way" of doing things. This makes it easier to maintain in the future by people who know the STL but who may not know your special class which behaves differently than the STL collections.

3) It is more extensible. A properly written functor doesn't care what kind of collection it acts upon. If for some reason you one day want to use a list instead of a vector, if you're using functors it is a much simpler matter to refactor than to reimplement a new special list-derived class.

4) It is overall a simpler design, and therefore less susceptible to defects and easier to maintain.

John Dibling
The derived class do not touch the vector-manipulation algorithms, but merely add some information such as "element width/height" for a vector of same-sized images.Thanks for sharing the wisdom, though.
sylvainulg
+7  A: 

It can be safe to have a public base class with a non-virtual destructor but behaviour is undefined if someone allocates an instance of your class with new, refers to it with a vector<...>*, and then deletes it using that pointer without casting it back to a pointer to your class. So users of your class must know not to do that. The surest way to stop them is not to give them the opportunity, hence the compiler warning.

To deal with this issue without having to impose such odd conditions on your users, the best advice is that for public base classes in C++, the destructor should be either public and virtual, or protected and non-virtual (http://www.gotw.ca/publications/mill18.htm, guideline #4). Since the destructor of std::vector is neither, that means it shouldn't be used as a public base class.

If all you want is to define some additional operations on vectors, then that's what free functions are for in C++. What's so great about the . member-call syntax anyway? Most of <algorithm> consists of additional operations on vector and other containers.

If you want to create, for example, a "vector with a maximum size limit", which will provide the entire interface of vector with modified semantics, then actually C++ does make that slightly inconvenient, compared with languages where inheritance and virtual calls are the norm. Simplest is to use private inheritance and then for the member functions of vector that you don't want to change, bring them into your class with using:

#include <vector>
#include <iostream>
#include <stdexcept>

class myvec : private std::vector<int> {
    size_t max_size;
  public:
    myvec(size_t m) : max_size(m) {}
    // ... other constructors

    void push_back(int i) {
        check(size()+1);
        std::vector<int>::push_back(i);
    }
    // ... other modified functions

    using std::vector<int>::operator[];
    // ... other unmodified functions

  private:
    void check(size_t newsize) {
        if (newsize > max_size) throw std::runtime_error("limit exceeded");
    }
};

int main() {
    myvec m(1);
    m.push_back(3);
    std::cout << m[0] << "\n";
    m.push_back(3); // throws an exception
}

You still have to be careful, though. The C++ standard doesn't guarantee which functions of vector call each other, or in what ways. Where those calls do occur, there's no way for my vector base class to call the overload in myvec, so my changed functions simply won't apply -- that's non-virtual functions for you. I can't just overload resize() in myvec and be done with it, I have to overload every function that changes the size and make them all call check (directly or by calling each other).

You can deduce from restrictions in the standard that some things are impossible: for example, operator[] can't change the size of the vector, so in my example I'm safe to use the base-class implementation, and I only have to overload functions which might change the size. But the standard won't necessarily provide guarantees of that kind for all conceivable derived classes.

In short, std::vector is not designed to be a base class, and hence it may not be a very well-behaved base class.

Of course, if you use private inheritance then you can't pass myvec to a function that requires a vector. But that's because it isn't a vector - its push_back function doesn't even have the same semantics as a vector, so we're on dodgy grounds with LSP, but more importantly non-virtual calls to functions of vector ignore our overloads. That's OK if you do things the way the standard libraries anticipate - use lots of templates, and pass iterators rather than collections. It's not OK if you want virtual function calls, because quite aside from the fact that vector doesn't have a virtual destructor, it doesn't have any virtual functions.

If you actually want dynamic polymorphism with standard containers (that is, you want to do vector<int> *ptr = new myvec(1);), then you're entering "thou shalt not" territory. The standard libraries can't really help you.

Steve Jessop
Thanks for putting it clearly and in details. I've made std::vector a private base class and add some friend class so that both the code within my library works fine as previously and code in clients of that library no longer rely on the wisdom that "We sure wouldn't downcast PhotoAlbum to std::vector<Photo>.
sylvainulg