Thou shalt not inherit from std::vector

68,088

Solution 1

Actually, there is nothing wrong with public inheritance of std::vector. If you need this, just do that.

I would suggest doing that only if it is really necessary. Only if you can't do what you want with free functions (e.g. should keep some state).

The problem is that MyVector is a new entity. It means a new C++ developer should know what the hell it is before using it. What's the difference between std::vector and MyVector? Which one is better to use here and there? What if I need to move std::vector to MyVector? May I just use swap() or not?

Do not produce new entities just to make something to look better. These entities (especially, such common) aren't going to live in vacuum. They will live in mixed environment with constantly increased entropy.

Solution 2

The whole STL was designed in such way that algorithms and containers are separate.

This led to a concept of different types of iterators: const iterators, random access iterators, etc.

Therefore I recommend you to accept this convention and design your algorithms in such way that they won't care about what is the container they're working on - and they would only require a specific type of iterator which they'd need to perform their operations.

Also, let me redirect you to some good remarks by Jeff Attwood.

Solution 3

The main reason for not inheriting from std::vector publicly is an absence of a virtual destructor that effectively prevents you from polymorphic use of descendants. In particular, you are not allowed to delete a std::vector<T>* that actually points at a derived object (even if the derived class adds no members), yet the compiler generally can't warn you about it.

Private inheritance is allowed under these conditions. I therefore recommend using private inheritance and forwarding required methods from the parent as shown below.

class AdVector: private std::vector<double>
{
    typedef double T;
    typedef std::vector<double> vector;
public:
    using vector::push_back;
    using vector::operator[];
    using vector::begin;
    using vector::end;
    AdVector operator*(const AdVector & ) const;
    AdVector operator+(const AdVector & ) const;
    AdVector();
    virtual ~AdVector();
};

You should first consider refactoring your algorithms to abstract the type of container they are operating on and leave them as free templated functions, as pointed out by majority of answerers. This is usually done by making an algorithm accept a pair of iterators instead of container as arguments.

Solution 4

If you're considering this, you've clearly already slain the language pedants in your office. With them out of the way, why not just do

struct MyVector
{
   std::vector<Thingy> v;  // public!
   void func1( ... ) ; // and so on
}

That will sidestep all the possible blunders that might come out of accidentally upcasting your MyVector class, and you can still access all the vector ops just by adding a little .v .

Solution 5

What are you hoping to accomplish? Just providing some functionality?

The C++ idiomatic way to do this is to just write some free functions that implement the functionality. Chances are you don't really require a std::vector, specifically for the functionality you're implementing, which means you're actually losing out on reusability by trying to inherit from std::vector.

I would strongly advise you to look at the standard library and headers, and meditate on how they work.

Share:
68,088

Related videos on Youtube

Armen Tsirunyan
Author by

Armen Tsirunyan

Updated on November 20, 2020

Comments

  • Armen Tsirunyan
    Armen Tsirunyan over 3 years

    Ok, this is really difficult to confess, but I do have a strong temptation at the moment to inherit from std::vector.

    I need about 10 customized algorithms for vector and I want them to be directly members of the vector. But naturally I want also to have the rest of std::vector's interface. Well, my first idea, as a law-abiding citizen, was to have an std::vector member in MyVector class. But then I would have to manually reprovide all of the std::vector's interface. Too much to type. Next, I thought about private inheritance, so that instead of reproviding methods I would write a bunch of using std::vector::member's in the public section. This is tedious too actually.

    And here I am, I really do think that I can simply inherit publicly from std::vector, but provide a warning in the documentation that this class should not be used polymorphically. I think most developers are competent enough to understand that this shouldn't be used polymorphically anyway.

    Is my decision absolutely unjustifiable? If so, why? Can you provide an alternative which would have the additional members actually members but would not involve retyping all of vector's interface? I doubt it, but if you can, I'll just be happy.

    Also, apart from the fact that some idiot can write something like

    std::vector<int>* p  = new MyVector
    

    is there any other realistic peril in using MyVector? By saying realistic I discard things like imagine a function which takes a pointer to vector ...

    Well, I've stated my case. I have sinned. Now it's up to you to forgive me or not :)

    • SingleNegationElimination
      SingleNegationElimination over 13 years
      Re: std::vector<int>* p = new MyVector doesn't really work in the first place, because std::vector does not have a virtual destructor. There will most certainly be undefined behavior in the near future.
    • Jim Brissom
      Jim Brissom over 13 years
      So, you basically asking if it's ok to violate a common rule based on the fact that you are just too lazy to re-implement the container's interface? Then no, it is not. See, you can have the best of both worlds if you swallow that bitter pill and do it properly. Don't be that guy. Write robust code.
    • Crashworks
      Crashworks over 13 years
      Why is passing a pointer to vector in a function param unrealistic?
    • Simone
      Simone over 13 years
      Why can't you/don't want to add the functionality you need with non-member functions? To me, that would be the safest thing to do in this scenario.
    • UncleBens
      UncleBens over 13 years
      @Crashworks: Probably since nobody allocates vector's dynamically, no function that receives a pointer to one will try to delete it? Apart from that, the static type of the argument will be a vector, and that should be just fine.
    • sbi
      sbi over 13 years
      @Jim: std::vector's interface is quite huge, and when C++1x comes along, it will greatly expand. That's a lot to type and more to expand in a few years. I think this is a good reason to consider inheritance instead of containment - if one follow the premise that those functions should be members (which I doubt). The rule to not to derive from STL containers is that they aren't polymorphic. If you aren't using them that way, it doesn't apply.
    • josesuero
      josesuero over 13 years
      The real meat of the question is in the one sentence: "I want them to be directly members of the vector". Nothing else in the question really matters. Why do you "want" this? What is the problem with just providing this functionality as non-members?
    • Basilevs
      Basilevs over 13 years
      As I mentioned in a comment to Karl's answer, caching is hard to implement in non-members.
    • rafak
      rafak over 13 years
      Sorry for my ignorance, but lots of the classes I write don't have virtual destructors, I inherit from them, but don't use them polymorphically. What is special with std containers and inheriting from them?
    • Ankit Roy
      Ankit Roy over 12 years
      @rafak: Nothing's special about the standard containers in that respect. But it's suspect to publicly inherit from any class if you don't intend to use it polymorphically: if you're simply going to add convenience methods, it's better to write these as free functions, because then they can also be used with existing objects of the base type; if you're adding extra invariants, people prefer having D contain a B instead of inheriting from it to avoid the risk of "accidental polymorphism" (having a D object treated as a B and breaking those invariants), even though it's more work.
    • Disillusioned
      Disillusioned about 10 years
      I'm going to be blunt. Saying: "provide a warning in the documentation that this class should not be used polymorphically. I think most developers are competent enough to understand that this shouldn't be used polymorphically anyway" is really pie-in-the-sky wishful thinking. You'll create a fertile breeding ground for future problems by skimping on a few lines of code today. In the blink of an eye "other developers" will be ignoring the documentation and doing things they're "not supposed to". Don't forget you're also doing something you're "not supposed to". ;)
    • Josh C
      Josh C almost 10 years
      thou shall not misspell shall as shalt
    • ruakh
      ruakh over 9 years
      @JoshC: "Thou shalt" has always been more common than "thou shall", and it's also the version found in the King James Bible (which is generally what people are alluding to when they write "thou shalt not [...]"). What on Earth would lead you to call it a "misspelling"?
    • Iron Attorney
      Iron Attorney over 6 years
      Some of these arguments are based on other developers screwing up because they haven't understood your std::container-inherrited implementations properly... but if you are the sole developer and always expecting to be so, you can discount at least these arguments right?
  • Armen Tsirunyan
    Armen Tsirunyan over 13 years
    Oh, please, I know that, but in this particular case it would be VERY convenient that the new functionality be as members.
  • Karl Knechtel
    Karl Knechtel over 13 years
    I'm not convinced. Could you update with some of the proposed code to explain why?
  • Armen Tsirunyan
    Armen Tsirunyan over 13 years
    look, for example, apart from back() and front() I want also to provide middle(), and I don't want it to look like middle(v), but v.middle()
  • snemarch
    snemarch over 13 years
    @Armen: apart from the aesthetics, are there any good reasons?
  • UncleBens
    UncleBens over 13 years
    @Armen: Better aesthetics, and greater genericity, would be to provide free front and back functions too. :) (Also consider the example of free begin and end in C++0x and boost.)
  • Armen Tsirunyan
    Armen Tsirunyan over 13 years
    @UncleBens: Hmm... I really like your idea, you know? :)
  • Frank Osterfeld
    Frank Osterfeld over 13 years
    I still don't what's wrong with free functions. If you don't like the "aesthetics" of the STL, maybe C++ is wrong place for you, aesthetically. And adding some member functions won't fix it, as much other algorithms are still free functions.
  • Armen Tsirunyan
    Armen Tsirunyan over 13 years
    "You only need a virtual destructor if the sizes of the base class and the derived class are different nad/or you have virtual functions (which means a v-table)." This claim is practically correct, but not theoretically
  • Steve Fallows
    Steve Fallows over 13 years
    Your first sentence is true 100% of the time. :)
  • josesuero
    josesuero over 13 years
    @Armen: no, aesthetics are not a good reason. Code readability and consistency are good reasons. But it doesn't have to look "pretty". It just has to be clear and readable. And of course, aesthetics are subjective. Anyway, as suggested, a much better way to get a consistent syntax is to re-implement members (front(), back() and whatever else) as non-members. In my code I've defined free begin() and end() functions, partly for the more consistent syntax, and partly because then I can use them with arrays as well.
  • josesuero
    josesuero over 13 years
    Unfortunately, the second sentence isn't. He hasn't given it a lot of thought. Most of the question is irrelevant. The only part of it that shows his motivation is "I want them to be directly members of the vector". I want. No reason for why this is desirable. Which sounds like he has given it no thought at all.
  • josesuero
    josesuero over 13 years
    yep, in principle it is still undefined behavior.
  • hmuelner
    hmuelner over 13 years
    If you claim that this is undefined behaviour I would like to see a proof (quotation from the standard).
  • Jay
    Jay over 13 years
    @snemarch: Seconded. aesthetics are not a good reason. Code should run well, not look pretty for programmers.
  • hmuelner
    hmuelner over 13 years
    To be more specific: The defined behaviour is that the destructor of the derived class may not be called. As it has to be empty nothing undefined or bad happens.
  • Basilevs
    Basilevs over 13 years
    It is hard to cache a result of heavy operation in an external algorithm. Suppose you have to calculate a sum of all elements in the vector or to solve a polynomial equation with vector elements as coefficients. Those operations are heavy and laziness would be useful for them. But you can't introduce it without wrapping or inheriting from the container.
  • Ben Voigt
    Ben Voigt over 13 years
    @hmuelner: Unfortunately, Armen and jalf are correct on this one. From [expr.delete] in the C++0x FCD: <quote> In the first alternative (delete object), if the static type of the object to be deleted is different from its dynamic type, the static type shall be a base class of the dynamic type of the object to be deleted and the static type shall have a virtual destructor or the behavior is undefined.</quote>
  • Ben Voigt
    Ben Voigt over 13 years
    Which is funny, because I actually thought the behavior was dependent on the presence of a non-trivial destructor (specifically, that POD classes could be destroyed via a pointer-to-base).
  • Ankit Roy
    Ankit Roy over 13 years
    @Basilevs' comment is the first good reason I've ever heard for adding methods as opposed to free functions.
  • sehe
    sehe almost 13 years
    I love this answer. It gave me a profound new handle on C++ class design choices.
  • bruno nery
    bruno nery almost 12 years
    And exposing containers and algorithms? See Kos' answer above.
  • stinky472
    stinky472 over 11 years
    My only counterargument to this is that one must really know what he's doing to do this. For example, do not introduce additional data members into MyVector and then try to pass it in to functions that accept std::vector& or std::vector*. If there's any kind of copy assignment involved using std::vector* or std::vector&, we have slicing issues where the new data members of MyVector will not be copied. The same would be true of calling swap through a base pointer/reference. I tend to think any kind of inheritance hierarchy that risks object slicing is a bad one.
  • Viet
    Viet over 11 years
    @Basilevs: I had been almost convinced that external better than internal until I saw your comment. If one insists to have external generic function, then a caching policy & contract must be designed :)
  • Disillusioned
    Disillusioned about 10 years
    In my experience, the worst long-term damage is often caused by people who try something ill-advised, and "so far haven't experienced (read noticed) any problems with it".
  • cubuspl42
    cubuspl42 almost 10 years
    @Basilevs You can make your algorithm (function) data-oriented and take both container and cache as in/out arguments. That would even increase flexibility. (What if someone doesn't want cache? They can pass nullptr as cache, then. Or what if someone wants to store it separately from vector data? No problem, either.)
  • Armen Tsirunyan
    Armen Tsirunyan over 9 years
    Great answer. Welcome to SO!
  • André Fratelli
    André Fratelli about 8 years
    std::vector's destructor is not virtual, therefore you should never inherit from it
  • Justin Time - Reinstate Monica
    Justin Time - Reinstate Monica almost 8 years
    @AndréFratelli Or at the very least, if you do inherit from it, you should do so as an internal class that other programmers can't use (and inadvertently use polymorphically) without mucking about in your source code, and make sure you either never use it polymorphically, never create a MyVector on the heap, or only use a function that explicitly takes a pointer or reference to a MyVector to delete it. That would be bad style, though, so one would need a very good reason to do it.
  • Justin Time - Reinstate Monica
    Justin Time - Reinstate Monica almost 8 years
    Basically, coding styles generally exist for a reason. If one wants to go against the generally accecpted style in situations like this, one should: Have a very good reason for doing so, be entirely aware of the risks involved in doing so, make sure that all of their code takes these risks into account, and actively prevent others from using their code in a manner that doesn't take the risks into account. It's usually more trouble than it's worth, especially since it indirectly involves a lot more effort than just having a private std::vector and exposing its interface as needed.
  • Justin Time - Reinstate Monica
    Justin Time - Reinstate Monica almost 8 years
    @cubuspl42 This also opens up the possibility of providing a default caching solution that is used unless the user explicitly specifies otherwise, similar to what the standard does for allocation with std::allocator. Doing so would allow the programmer to make a cache that performs optimally in most situations so that the user doesn't need to make their own, while also allowing them to use their own custom cache when it would be more efficient.
  • Graham Asher
    Graham Asher over 7 years
    I created a class which publicly inherited std::vector for this reason: I had old code with a non-STL vector class, and I wanted to move to STL. I reimplemented the old class as a derived class of std::vector, allowing me to continue using the old function names (e.g., Count() rather than size()) in old code, while writing new code using the std::vector functions. I did not add any data members, therefore std::vector's destructor worked fine for objects created on the heap.
  • Peter - Reinstate Monica
    Peter - Reinstate Monica over 7 years
    IIUC, the absence of a virtual destructor is only a problem if the derived class allocates resources which must be freed upon destruction. (They wouldn't be freed in a polymorphic use case because a context unknowingly taking ownership of a derived object via pointer to base would only call the base destructor when it is time.) Similar issues arise from other overridden member functions, so care must be taken that the base ones are valid to call. But absent additional resources, are there any other reasons?
  • Peter - Reinstate Monica
    Peter - Reinstate Monica over 7 years
    vector's allocated storage is not the issue -- after all, vector's destructor would be called all right through a pointer to vector. It's just that the standard forbids deleteing free store objects through a base class expression. The reason is surely that the (de)allocation mechanism may try to infer the size of the memory chunk to free from delete's operand, for example when there are several allocation arenas for objects of certain sizes. This restriction does, afaics, not apply to normal destruction of objects with static or automatic storage duration.
  • Peter - Reinstate Monica
    Peter - Reinstate Monica over 6 years
    @DavisHerring I think we agree there :-).
  • Peter - Reinstate Monica
    Peter - Reinstate Monica over 6 years
    @DavisHerring Ah, I see, you refer to my first comment -- there was an IIUC in that comment, and it ended in a question; I saw later that indeed it is always forbidden. (Basilevs had made a general statement, "effectively prevents", and I wondered about the specific way in which it prevents.) So yes, we agree: UB.
  • ThomasMcLeod
    ThomasMcLeod over 5 years
    @Basilevs That must have been inadvertent. Fixed.
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont almost 5 years
    @graham if UB is fine, sure.
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont almost 5 years
    @andre you shall not allocate a std vector using new on the heap nor delete it via a raw pointer; you shall not use a unique ptr to a vector. Pointer-based ownership requires virtual dtors; other ownership semantics do not.
  • Graham Asher
    Graham Asher almost 5 years
    @Yakk-AdamNevraumont: I assume that by UB you mean undefined behaviour. There was none.
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont almost 5 years
    @GrahamAsher If you delete an object through a pointer to base, and the destructor is not virtual, your program is exhibiting undefined behavior. One possible result of undefined behavior is "it worked fine in my tests". Another is that it emails your grandmother your web browsing history. Both are compliant with the C++ standard. It changing from one to the other with point releases of compilers, OSs, or the phase of the moon is also compliant.
  • Graham Asher
    Graham Asher almost 5 years
    @Yakk-AdamNevraumont If you delete an object through a pointer to base and the destructor is not virtual but the derived class adds no new data members or behaviour that necessitates a different destructor, the behaviour is not undefined. The base class destructor is called, and works correctly.
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont almost 5 years
    @GrahamAsher No, whenever you delete any object through a pointer to base without a virtual destructor, that is undefined behavior under the standard. I understand what you think is happening; you just happen to be wrong. "the base class destructor is called, and it works" is one possible symptom (and the most common) of this undefined behavior, because that is the naive machine code that the compiler usually generates. This does not make it safe nor a great idea to do.
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont almost 5 years
    @GrahamAsher open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4713.pdf [Delete] 8.5.2.5/3: "In a single-object delete expression, if the static type of the object to be deleted is different from its dynamic type, the static type shall be a base class of the dynamic type of the object to be deleted and the static type shall have a virtual destructor or the behavior is undefined". -- it isn't even undefined by omission. It is explicitly undefined behavior. The meaning of a C++ program is not defined by the assembly it produces.
  • Graham Asher
    Graham Asher almost 5 years
    @Yakk-AdamNevraumont The standard is either unclear, incomplete or wrong on this point. When the compiler generates destructor code via a pointer to base, in a class with a non-virtual destructor, it has access only to the base class destructor, and thus (in the special case of a derived class adding no data members and having an empty destructor of its own) has no information which it can use to create undefined or incorrect behaviour, or to call the wrong destructor code. It is guaranteed to call the base destructor, and (in this special case) that is the right thing to do.
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont almost 5 years
    @graham C++ is not defined by the assembly code it generates. The standard is clear, complete and by definition normative; it defines what is C++. If you want to change the standard, make a proposal. Until then, your code has behaviour explictly and clearly not defined by the standard. I get it. Thinking C++ is defined by the code it generates is a common error. But until you understand that fundamental mistake, you will continue to be screwed over and probably angry when ((int)(unsigned)(int)-1) >= 0 is optimized into true and a whole myriad of other things. Including this mistake.
  • Andre Holzner
    Andre Holzner over 4 years
  • 0xB00B
    0xB00B over 2 years
    @jalf I agree, readability and consistency are good reasons to have methods. So I think std::list should have operator [] as well. Also vec.contains(stuff) would be more readable than std::find(vec.begin() vec.end(), stuff) != vec.end()
  • Graham Asher
    Graham Asher almost 2 years
    If inheriting from std::vector is good enough for Bjarne Stroustrup, it's good enough for me. See JL Borges's comment here: cplusplus.com/forum/general/156923. He quotes Stroustrup's Tour of C++ (draft), section 4.4.1.2, in which he creates a range-checking vector class Vec which inherits publicly from std::vector. I similarly create a class which inherits from std::vector and adds no data members.