Inheritance on C++ Classes that contain another Derived classes

Question

Having the next C++ classes as a simplifiacation of the issue:

struct Car
{
    virtual int get_price() = 0;
};

struct ExpensiveCar: public Car
{
    int get_price( ) {/*..*/ }
    void apply_turbo( ){/*..*/};
};

struct CheapCar: public Car
{
    int get_price( ) {/*..*/}
};

struct CarRetailer
{
    virtual std::vector<Car*> get_cars( ) = 0;
};


struct ExpensiveCarsRetailer : public CarRetailer
{
    virtual std::vector<Car*> get_cars( ) { /*..*/ }
    std::vector<ExpensiveCar*> get_cars_for_exhibitions( );
};

struct CheapCarsRetailer : public CarRetailer
{
    virtual std::vector<Car*> get_cars( ) { /*..*/ }
    std::vector<CheapCar*> get_second_hand_cars( );
};

The rules are: Expensive Cars are only sold in ExpensiveCarsRetailers (similar for Cheap Cars ). Cheap cars do not have turbo and expensive cars are not sold in second hand.

The issue I face here is the coupling of classes that contains inherited classes as well. Therefore, if ExpensiveCarRetailer inherits from CarRetailer, it will need to implement virtual std::vector<Car*> get_cars( ) that is actually returning a vector of Car*, however, internally ExpensiveCarRetailer only created objects of ExpensiveCar. Furthermore, the get_cars_for_exhibitions() is not included in the public interface CarRetailertherefore, it can return a std::vector<ExpensiveCar*> instead.

The mixture in the API ( returning vector of Car* and ExpensiveCar* ) is very ugly, and so is the code that the user needs to write to use the apply_turbo( ) function of the list of cars from a specific ExpesiveCarsRetailer.

ExpensiveCarsRetailer ferrari;

std::vector<Car*> car = ferrari.get_cars();
ExpensiveCar* expensive_car;
for( int i = 0; i < car.size( ); ++i)
{
expensive_car = dynamic_cast<ExpensiveCar*>(car[i]);
expensive_car->apply_turbo();
}

I am sure I am missing some design pattern that helps in this situation, where to trees of class inheritances are coupled in a way that the abstract class of one of the inheritance tress need to return a vector (or set, list, etc ) of classes on the other inheritance tree. I am trying to avoid the dynamic casting as much as possible.

I also thought about making CarRetailer a templated class, therefore:

template<typename T>
    struct CarRetailer
    {
        virtual std::vector<T*> get_cars( ) = 0;
    };

And then make:

struct ExpensiveCarRetailer: public CarRetailer<ExpensiveCar>
{
...
}

But I don't think this would work since, for example, the CarRetailer can also start selling motorbikes ( similar structure as cars ) or bikes etc ( always applying the Expensive/Cheap pattern ) and eventually the number of template classes that need to be defined in CarRetailer will be huge.

Answer 1

Why not have a non-virtual function to get the cars just like you have done with get_cars_for_exhibitions? In the example you have shown the caller knows they have an ExpensiveCarsRetailer so they do not need the polymorphism. I assume you do need the inheritance and polymorphism in other contexts but it would be easy enough to convert from a vector of ExpensiveCar pointers to a vector of Car pointers to fulfil the CarRetailer interface:

template<typename T>
using Ptrs = std::vector<std::unique_ptr<T>>;

template<class T, class U>
Ptrs<U> upcast(Ptrs<T> input) {
  return Ptrs<U>(std::make_move_iterator(input.begin()),  
                 std::make_move_iterator(input.end()));
}
struct CarRetailer {
  virtual Ptrs<Car> getCars() = 0;
};

struct ExpensiveCarsRetailer : CarRetailer {
  Ptrs<ExpensiveCar> getCarsImpl() { ... }
  Ptrs<Car> getCars() override { return upcast<Car>(getCarsImpl()); }
};

Then you can call the non-virtual function if you need a vector of ExpensiveCar pointers:

ExpensiveCarsRetailer ferrari;

auto expensive_cars = ferrari.getCarsImpl();
for (auto& expensive_car : expensive_cars)
  expensive_car->apply_turbo();

And in a polymorphic context you can call the virtual function:

CarRetailer* retailer = &ferrari;
auto cars = retailer->getCars();
for (auto& car : cars)
   std::cout << "Car price " << car->get_price() << "\n";

Answer 2

The whole concept of "coupling of classes that contains inherited classes as well" is called covariance. On Wikipedia one can find an extensive review of the subject: http://en.wikipedia.org/wiki/Covariance_and_contravariance_(computer_science). But let's get back to the practical example: by writing

struct CarRetailer
{
    virtual std::vector<Car*> get_cars( ) = 0;
};

you are promising too much, i.e. that the get_cars() function will return a vector which you can modify with whatever Car you want. That is probably not what you meaned:

CarRetailer* ferrari = new ExpensiveCarsRetailer();
auto niceCars ferrari->get_cars();
niceCars.push_back(new Car{"Trabant"}); // you promised in the declaration that it was possible!

The solution is reducing the "promises" you do in the root class, returning a range which cannot be altered with "extraneous" objects. A read-only container would be nice, but alas, C++ is not (yet?) smart enough to support convariance on it:

struct CarRetailer
{
    virtual const std::vector<const Car*> get_cars( ) = 0;
};

struct ExpensiveCarsRetailer : public CarRetailer 
{
    const std::vector<const ExpensiveCar*> get_cars( ) = 0;
    // Alas, it won't override
};

But ranges (i.e. pointers pairs, in the hope that C++17 will offer something better) will do:

struct CarRetailer
{
    virtual Car* const cars_begin( ) = 0;
    virtual Car* const cars_end( ) = 0;
};

struct ExpensiveCarsRetailer : public CarRetailer 
{
    ExpensiveCar* const cars_begin( ) override {return cars->begin();}
    ExpensiveCar* const cars_end( ) override {return cars->end();}

    private:
    vector<ExpensiveCar>* cars; 
};

(note: I did not test my code, so please forgive possible mistakes. The concept should be clear, though)

This interface may seem uglier than the original one, but I am of the opposite opinion, since it integrates perfectly with the powerful <algorithm> C++ library and facilitates writing code in modern style. Here a simple example:

any_of(dealer.cars_begin( ), dealer.cars_end( ),
       [](const auto& car) -> bool {return car.hasScratch();}
) ? complain() : congratulate();

The biggest shortcoming of this design is that the CarDealer class has to own the *cars container and manage its existence, i.e. it must care that the pointers it returns stay alive. This is made difficult by the fact that one cannot return smart pointers, since C++ does not support covariance on them. Additionally one could end up having to mantain a container of vector<Car>*, if the cars_begin and cars_end functions are supposed to generate new collections on repeated invocations. So it's up to you to balance pros and cons of my proposal in your concrete use case.

On a personal note, if I had the same issue I would use templated classes and avoid inheritance at all: such kind of problems IMHO well illustrates why OOP sometimes complicates things instead of simplifying them.

EDIT

If I understand well why you feel templated Dealers would not be indicated for your needs, I think this proposal should be more fit:

struct ExpensiveCarsRetailer /* not derived, not templated */
{
    std::vector<ExpensiveCar> get_cars( ) { /*..*/ }
    // you can also return a vector of pointers or of unique_pointers, as you feel like.
};

struct CheapCarsRetailer /* not derived, not templated */
{
    std::vector<CheapCar> get_cars( );
};

Instead of overloading, templated functions are used:

template <typename T> print_car_table(T dealer) {
    // This will work on both Expensive and Cheap dealers

    // Not even a hierarchy for the Car classes is needed:
    // they can be independent structs, like the Dealer ones

    auto cars = dealer.get_cars();

    for (const auto& car : cars) { std::cout << car.name() << "\t" << car.color() << "\n"; }
}

template <typename T> apply_turbo(T dealer) {
    // This will work if dealer is an ExpensiveDealer,
    // and fail on compile time if not, as wanted

    auto cars = dealer.get_cars();

    for (auto& car : cars) { car.apply_turbo(); }
}

The big advantage of this system is that you do not even have to plan the interface in advance, and each class can implement just the methods you need. So if in future you add a CarMuseum, you can decide to implement get_cars() to get print_car_table(T) work with it, but you are free not to create any other method. With inheritance, you are instad forced to implement all the functions declared in the base interface, or create many fragmented interfaces (class CheapDealer : public HasACarList, public HasAPriceList, /* yuck ...*/). .

The disadvantages of this templated design are the consequence of the Dealer classes not being relatives. This means you cannot create a vector<Dealer> nor have Dealer* (even if you made them derive from a tiny common interface, you could not call get_cars() through a pointer to such interface).

For completeness, I would point out the opposite, dynamic, solution:

struct Car {
    virtual int get_price() = 0;
    virtual void apply_turbo( ) = 0;
};

struct CheapCar: public Car
{
    int get_price( ) {/*..*/}
    void apply_turbo( ){throw OperationNonSupportedException();};
};

It does not feel very idiomatic C++, does it? I find it quite inelegant, but still I think this design should be evaluated before discarding it. Advantages and disadvantages are more or less the opposite of the templated solution.

Finally, I guess that the Visitor pattern or Scala (offering very powerful covariance facilities) could offer other alternative solutions to your problem. But I have no experience with both of them, so I leave them to others to illustrate.