Is it okay to inherit implementation from STL containers, rather than delegate?
Asked Answered
F

8

88

I have a class that adapts std::vector to model a container of domain-specific objects. I want to expose most of the std::vector API to the user, so that they may use familiar methods (size, clear, at, etc...) and standard algorithms on the container. This seems to be a reoccurring pattern for me in my designs:

class MyContainer : public std::vector<MyObject>
{
public:
   // Redeclare all container traits: value_type, iterator, etc...

   // Domain-specific constructors
   // (more useful to the user than std::vector ones...)

   // Add a few domain-specific helper methods...

   // Perhaps modify or hide a few methods (domain-related)
};

I'm aware of the practice of preferring composition to inheritance when reusing a class for implementation -- but there's gotta be a limit! If I were to delegate everything to std::vector, there would be (by my count) 32 forwarding functions!

So my questions are... Is it really so bad to inherit implementation in such cases? What are the risks? Is there a safer way I can implement this without so much typing? Am I a heretic for using implementation inheritance? :)

Edit:

What about making it clear that the user should not use MyContainer via a std::vector<> pointer:

// non_api_header_file.h
namespace detail
{
   typedef std::vector<MyObject> MyObjectBase;
}

// api_header_file.h
class MyContainer : public detail::MyObjectBase
{
   // ...
};

The boost libraries seem to do this stuff all the time.

Edit 2:

One of the suggestions was to use free functions. I'll show it here as pseudo-code:

typedef std::vector<MyObject> MyCollection;
void specialCollectionInitializer(MyCollection& c, arguments...);
result specialCollectionFunction(const MyCollection& c);
etc...

A more OO way of doing it:

typedef std::vector<MyObject> MyCollection;
class MyCollectionWrapper
{
public:
   // Constructor
   MyCollectionWrapper(arguments...) {construct coll_}

   // Access collection directly
   MyCollection& collection() {return coll_;} 
   const MyCollection& collection() const {return coll_;}

   // Special domain-related methods
   result mySpecialMethod(arguments...);

private:
   MyCollection coll_;
   // Other domain-specific member variables used
   // in conjunction with the collection.
}
Fishtail answered 9/1, 2010 at 20:54 Comment(7)
Oh goody! Another chance to push my blog at punchlet.wordpress.com - basically, write free functions, and forget the "more OO" wrapper approach. It is not more OO - if it were it would use inheritence, which you probably shouldn't in this case. Remember OO != class.Paronomasia
@Neil: But, but.. global functions are evil!!! Everything is an object! ;)Fishtail
They won't be global if you put them in a namespace.Paronomasia
@Neil: I know, I know. Was just goofing around. Interesting blog. The "special functions" I'm talking about here though are not really general purpose and only make sense in my particular app. That's why I'm hesitant to make them free functions and would like to bundle them in a class. I've revised my last code sample.Fishtail
If you really do want to expose the whole interface of vector, then it's probably better in C++ to use composition, and expose a reference to the vector via a getter (with const and non-const versions). In Java you'd just inherit, but then in Java some numpty won't come along, ignore your documentation, delete your object through the wrong pointer (or inherit again and mess it up), and then complain. For a limited audience maybe, but if users might be dynamic-polymorphism freaks, or recently ex-Java programmers, you're designing an interface that you can be pretty sure they'll misunderstand.Cubature
You can't protect against people completely ignoring documentation. I'd not be surprised to find out such misuse causes just as many problems in Java as in C++.Youlandayoulton
IMO It is fine. No one should be making pointers to vectors because vectors already store their data dynamically and there is no polymorphism possible. The only real reason to use a pointer is std::shared_ptr and that is designed to correctly destroy derived types that do not have virtual destructors.Waterline
Y
78

The risk is deallocating through a pointer to the base class (delete, delete[], and potentially other deallocation methods). Since these classes (deque, map, string, etc.) don't have virtual dtors, it's impossible to clean them up properly with only a pointer to those classes:

struct BadExample : vector<int> {};
int main() {
  vector<int>* p = new BadExample();
  delete p; // this is Undefined Behavior
  return 0;
}

That said, if you're willing to make sure you never accidentally do this, there's little major drawback to inheriting them—but in some cases that's a big if. Other drawbacks include clashing with implementation specifics and extensions (some of which may not use reserved identifiers) and dealing with bloated interfaces (string in particular). However, inheritance is intended in some cases, as container adapters like stack have a protected member c (the underlying container they adapt), and it's almost only accessible from a derived class instance.

Instead of either inheritance or composition, consider writing free functions which take either an iterator pair or a container reference, and operate on that. Practically all of <algorithm> is an example of this; and make_heap, pop_heap, and push_heap, in particular, are an example of using free functions instead of a domain-specific container.

So, use the container classes for your data types, and still call the free functions for your domain-specific logic. But you can still achieve some modularity using a typedef, which allows you to both simplify declaring them and provides a single point if part of them needs to change:

typedef std::deque<int, MyAllocator> Example;
// ...
Example c (42);
example_algorithm(c);
example_algorithm2(c.begin() + 5, c.end() - 5);
Example::iterator i; // nested types are especially easier

Notice the value_type and allocator can change without affecting later code using the typedef, and even the container can change from a deque to a vector.

Youlandayoulton answered 9/1, 2010 at 21:1 Comment(0)
M
41

You can combine private inheritance and the 'using' keyword to work around most of the problems mentioned above: Private inheritance is 'is-implemented-in-terms-of' and as it is private you cannot hold a pointer to the base class

#include <string>
#include <iostream>

class MyString : private std::string
{
public:
    MyString(std::string s) : std::string(s) {}
    using std::string::size;
    std::string fooMe(){ return std::string("Foo: ") + *this; }
};

int main()
{
    MyString s("Hi");
    std::cout << "MyString.size(): " << s.size() << std::endl;
    std::cout << "MyString.fooMe(): " << s.fooMe() << std::endl;
}
Monteiro answered 9/1, 2010 at 21:33 Comment(4)
I cannot help to mention that private inheritance is still inheritance and thus a stronger relationship than composition. Notably, it means that changing the implementation of your class is necessarily going to break the binary compatibility.Jacobus
Private inheritance and private data members both break binary compatibility when they change, and, except for friends (which should be few), it's usually not hard to switch between them---which is used is often dictated by implementation details. Also see the "base-from-member idiom".Youlandayoulton
For the curious - Base-from-Member Idiom: en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Base-from-MemberFishtail
@MatthieuM. Breaking ABI is not a problem at all for most applications. Even some libraries live without Pimpl for better performance.Tabriz
C
16

As everyone has already stated, STL containers do not have virtual destructors so inheriting from them is unsafe at best. I've always considered generic programming with templates as a different style of OO - one without inheritance. The algorithms define the interface that they require. It is as close to Duck Typing as you can get in a static language.

Anyway, I do have something to add to the discussion. The way that I have created my own template specializations previously is to define classes like the following to use as base classes.

template <typename Container>
class readonly_container_facade {
public:
    typedef typename Container::size_type size_type;
    typedef typename Container::const_iterator const_iterator;

    virtual ~readonly_container_facade() {}
    inline bool empty() const { return container.empty(); }
    inline const_iterator begin() const { return container.begin(); }
    inline const_iterator end() const { return container.end(); }
    inline size_type size() const { return container.size(); }
protected: // hide to force inherited usage only
    readonly_container_facade() {}
protected: // hide assignment by default
    readonly_container_facade(readonly_container_facade const& other):
        : container(other.container) {}
    readonly_container_facade& operator=(readonly_container_facade& other) {
        container = other.container;
        return *this;
    }
protected:
    Container container;
};

template <typename Container>
class writable_container_facade: public readable_container_facade<Container> {
public:
    typedef typename Container::iterator iterator;
    writable_container_facade(writable_container_facade& other)
        readonly_container_facade(other) {}
    virtual ~writable_container_facade() {}
    inline iterator begin() { return container.begin(); }
    inline iterator end() { return container.end(); }
    writable_container_facade& operator=(writable_container_facade& other) {
        readable_container_facade<Container>::operator=(other);
        return *this;
    }
};

These classes expose the same interface as an STL container. I did like the effect of separating the modifying and non-modifying operations into distinct base classes. This has a really nice effect on const-correctness. The one downside is that you have to extend the interface if you want to use these with associative containers. I haven't run into the need though.

Chacon answered 9/1, 2010 at 22:50 Comment(2)
Nice! I might just use that. But others have made rethink about the idea of adapting containers, so maybe I won't use it. :)Fishtail
That being said, heavy template programming can lead to every bit as bad spaghetti code, massive libraries, poor isolation of functionality, and compile time errors that are unintelligible.Peculiar
H
6

Virtual dtors aside, the decision to inherit versus contain should be a design decision based the class you are creating. You should never inherit container functionality just because its easier than containing a container and adding a few add and remove functions that seem like simplistic wrappers unless you can definitively say that the class you are creating is a kind-of the container. For instance, a classroom class will often contain student objects, but a classroom isn't a kind of list of students for most purposes, so you shouldn't be inheriting from list.

Harleigh answered 9/1, 2010 at 21:6 Comment(0)
L
5

In this case, inheriting is a bad idea: the STL containers do not have virtual destructors so you might run into memory leaks (plus, it's an indication that STL containers are not meant to be inherited in the first place).

If you just need to add some functionality, you can declare it in global methods, or a lightweight class with a container member pointer/reference. This off course doesn't allow you to hide methods: if that is really what you are after, then there's no other option then redeclaring the entire implementation.

Lisle answered 9/1, 2010 at 21:1 Comment(8)
You can still hide methods by not declaring them in the header and instead only in the implementation, by making them non-public static methods in a dummy class (from which you can give friendship, and this works for templates that must be header-only), or by putting them in a "detail" or similarly named namespace. (All three work just as well as conventional private methods.)Youlandayoulton
I fail to understand how you think can hide a method of 'vector' by not declaring it in your header. Its already declared in vector.Harleigh
Jherico: Are you talking to me or stijn? Either way, I think you've misunderstood one of us.Youlandayoulton
@roger I second Jherico and don't think I understand you: are you talking about hiding methods from std::vector or from something else? Also how does putting a method in another namespace make it hidden? As long as declared in a header anyone has access to, it's not really hidden in a way the private keyword hides?Lisle
stijn: That's what I was pointing out about private access, it's not really hidden either, as anyone with access to the header can read the source or use -Dprivate=public on the compiler commandline. Access specifiers like private are mostly documentation, which just happens to be enforced.Youlandayoulton
stijn: Perhaps I was the one that misunderstood you, as I was talking about the global methods you mentioned and how you can "hide" implementation details without a class.Youlandayoulton
@Roger: what? They don't even need access to the header, they can reverse-engineer your binaries and directly manipulate your objects. Access specifiers are mostly documentation in the same sense that types are mostly documentation, because people can always reinterpret_cast pointers. What do you have to do before something's actually a language feature, hide it in ring 0? ;-)Cubature
Steve: While you make an important point that all those implementing DRM and crypto need to be aware of... ;) I'm not saying it's any less useful of a language feature just because it's documentation---as good documentation is always useful!---people just shouldn't expect more out of it than they get.Youlandayoulton
F
1

It is easier to do:

typedef std::vector<MyObject> MyContainer;
Flooded answered 9/1, 2010 at 21:2 Comment(1)
I understand, but I'm looking to do: typedef (std::vector<MuObject> + mods) MyContainer consisely.Fishtail
V
1

The forwarding methods will be inlined away, anyhow. You will not get better performance this way. In fact, you will likely get worse performance.

Visitation answered 9/1, 2010 at 22:5 Comment(0)
P
0

Always consider composition over inheritance.

Consider the case:

class __declspec(dllexport) Foo : 
  public std::multimap<std::string, std::string> {};

Then symbols of std::multimap will be exported into your dll, which may cause compilation error "std::multimap already defined".

Purkey answered 15/2, 2022 at 3:5 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.