How to deal with pointless methods for types contained in a variant in c++ 17

Question

I am dealing with a case in which a certain container class is required to hold a variant of custom classes (not least to collect instances of such in a vector). These in turn are interrelated to one another. In the code example, the types in this variant are Bird and Fish, and the container class is AnimalContainer (for the complete, working code, see below).

Incomplete class overview:

using namespace std;
using uint = unsigned int;

class Animal {      
    protected:
        uint length_;
};

class Fish : public Animal {   
    private:
        uint depths_of_dive_;
};

class Bird : public Animal {   
    private:
        uint wing_span_;
};

class AnimalContainer {
    private:
        variant<Bird, Fish> the_animal_;
};

Now (ignoring penguins and some other birds), birds can usually not dive, and fishes do not have wings (haven't heard of any at least). However, the code should provide the possibility to request the wing_span_ via an instance a of the AnimalContainer class using a.WingSpan(), should this animal be a Bird, as well as the depth_of_dive_ using a.DepthOfDive(), should it be a Fish. Additionally, for each Bird and Fish, a (physiologically unrealistic) weight can be estimated, that is a.EstimatedWeight() can be called.

Basically in order to avoid compiler errors, a method WingSpan() is added to the Fish class, and DepthOfDive() is added to the Bird class.

Adding these dummy methods can become very cumbersome, especially when more than two variants (here Fish and Bird) are involved, or when these classes contain many methods.

One possibility seems to overload the visitor for specific classes and to return some warning in all other cases (again using a generic lambda), but even though this improves the process a bit, it is quite cumbersome as well (see second code example below).

Do you have suggestions how to handle this in a more comprehensive way that requires less copy and paste? If you have general issues with this concept, advise is welcome as well.

By the way, the animal container class is later placed in another class which guides the user in order to avoid unintended calls of dummy functions.

First working code example

#include <variant>
#include <iostream>

using namespace std;
using uint = unsigned int;


class Animal {
    public:
        Animal(uint length) : length_{length} {}

        uint Length() { return length_; }

    protected:
        uint length_;
};

class Fish : public Animal {
    public:
        Fish(uint length, uint depths_of_dive) : Animal(length), depths_of_dive_{depths_of_dive} {}

        uint DepthOfDive() { return depths_of_dive_; }
        uint EstimatedWeight() { return length_ * length_; }

        uint WingSpan() { cerr << "Usually fishes do not have wings... "; return 0; }

    private:
        uint depths_of_dive_;
};

class Bird : public Animal {
    public:
        Bird(uint length, uint wing_span) : Animal(length), wing_span_{wing_span} {}

        uint WingSpan() { return wing_span_; }
        uint EstimatedWeight() { return wing_span_ * length_; }

        uint DepthOfDive() { cerr << "Usually birds can not dive... "; return 0; }

    private:
        uint wing_span_;
};

class AnimalContainer {
    public:
        AnimalContainer(Bird b) : the_animal_{b} {}
        AnimalContainer(Fish f) : the_animal_{f} {}

        uint Length() {
            return visit([] (auto arg) { return arg.Length(); }, the_animal_);
        }
        uint WingSpan() {
            return visit([] (auto arg) { return arg.WingSpan(); }, the_animal_);
        }
        uint DepthOfDive() {
            return visit([] (auto arg) { return arg.DepthOfDive(); }, the_animal_);
        }
        uint EstimatedWeight() {
            return visit([] (auto arg) { return arg.EstimatedWeight(); }, the_animal_);
        }


    private:
        variant<Bird, Fish> the_animal_;
};

int main()
{
    Fish f(2,3);
    Bird b(2,3);

    AnimalContainer a_1(f);
    AnimalContainer a_2(b);

    cout << a_1.Length() << ' ' << a_1.WingSpan() << ' ' << a_1.DepthOfDive() << ' ' << a_1.EstimatedWeight() << endl; 
    cout << a_2.Length() << ' ' << a_2.WingSpan() << ' ' << a_2.DepthOfDive() << ' ' << a_2.EstimatedWeight() << endl;

    return 0;
}

Second working code example

#include <variant>
#include <iostream>

using namespace std;
using uint = unsigned int;


class Animal {
    public:
        Animal(uint length) : length_{length} {}

        uint Length() { return length_; }

    protected:
        uint length_;
};

class Fish : public Animal {
    public:
        Fish(uint length, uint depths_of_dive) : Animal(length), depths_of_dive_{depths_of_dive} {}

        uint DepthOfDive() { return depths_of_dive_; }
        uint EstimatedWeight() { return length_ * length_; }

        // no more dummy function

    private:
        uint depths_of_dive_;
};

class Bird : public Animal {
    public:
        Bird(uint length, uint wing_span) : Animal(length), wing_span_{wing_span} {}

        uint WingSpan() { return wing_span_; }
        uint EstimatedWeight() { return wing_span_ * length_; }

        // no more dummy function

    private:
        uint wing_span_;
};

template<class... Ts> struct overloaded : Ts... { using Ts::operator()...; };
template<class... Ts> overloaded(Ts...) -> overloaded<Ts...>;

class AnimalContainer {
    public:
        AnimalContainer(Bird b) : the_animal_{b} {}
        AnimalContainer(Fish f) : the_animal_{f} {}

        uint Length() {
            return visit([] (auto arg) { return arg.Length(); }, the_animal_);
        }
        uint WingSpan() {
            return visit(overloaded { // now overloaded version
                [] (auto) { cerr << "This animal does not have wings... "; return uint(0); },
                [] (Bird arg) { return arg.WingSpan(); }}, the_animal_);
        }
        uint DepthOfDive() {
            return visit(overloaded { // now overloaded version
                [] (auto) { cerr << "This animal can not dive... "; return uint(0); },
                [] (Fish arg) { return arg.DepthOfDive(); }}, the_animal_);
        }
        uint EstimatedWeight() {
            return visit([] (auto arg) { return arg.EstimatedWeight(); }, the_animal_);
        }

    private:
        variant<Bird, Fish> the_animal_;
};

int main()
{
    Fish f(2,3);
    Bird b(2,3);

    AnimalContainer a_1(f);
    AnimalContainer a_2(b);

    cout << a_1.Length() << ' ' << a_1.WingSpan() << ' ' << a_1.DepthOfDive() << ' ' << a_1.EstimatedWeight() << endl; 
    cout << a_2.Length() << ' ' << a_2.WingSpan() << ' ' << a_2.DepthOfDive() << ' ' << a_2.EstimatedWeight() << endl;

    return 0;
}

Answer 1

First of all, let me say that I am very pleased to see a well-formulated question about the design from a new contributor. Welcome to StackOverflow! :)

As you properly mentioned, you have two choices: handle non-existing behavior in the concrete classes or in the container. Let us consider both options.

Concrete classes

This is usually done with help of inheritance and (dynamic) polymorphism, classic OOP approach. You shouldn't even have variant in this case, since variant is used for unrelated classes. It does not make much sense to use it when you already have a common base class.

Instead, define the whole interface you need in the base class as a set of virtual functions. A good practice is to have a pure interface on the top of the hierarchy. And then you can optionally have an intermediate (possibly abstract) class providing some default implementations. This will allow you not to think about unrelated concepts for each new derived animal, and to avoid some code duplication.

The code might look like this (not tested, just showing you the concept):

// Pure interface on top of the hierarchy
class IAnimal {
    public:
        virtual ~IAnimal() = default.

        virtual uint Length() const = 0;
        virtual uint DepthOfDive() const = 0;
        virtual uint EstimatedWeight() const = 0;
        virtual uint WingSpan() const = 0;
};

// Intermediate class with some common implementations 
class Animal : public IAnimal {
    public:
        Animal(uint length) : length_{length} {}

        // We know how to implement this on this level already, so mark this final
        // Otherwise it won't have much sense to have the length_ field
        uint Length() const final { return length_; }

        // Some of these should be overridden by the descendants
        uint DepthOfDive() const override { cerr << "This creature can not dive... "; return 0; }
        uint WingSpan() const override { cerr << "This creature does not have wings... "; return 0; }

    private:
        uint length_;  // Better make it private
};

class Fish : public Animal {
    public:
        Fish(uint length, uint depths_of_dive) : Animal(length), depths_of_dive_{depths_of_dive} {}

        uint DepthOfDive() const { return depths_of_dive_; }
        uint EstimatedWeight() const { return Length() * Length(); }

    private:
        uint depths_of_dive_;
};

class Bird : public Animal {
    public:
        Bird(uint length, uint wing_span) : Animal(length), wing_span_{wing_span} {}

        uint WingSpan() const { return wing_span_; }
        uint EstimatedWeight() const { return wing_span_ * Length(); }

    private:
        uint wing_span_;
};

using AnimalContainer = std::unique_ptr<IAnimal>;

Now instead of a unifying container, you can use a pointer to the base interface directly. Classic.

The container

Having a unifying container providing some common interface might make sense when you don't have a base class. Otherwise, you better retreat to the classic OOP described above. So, in this case you better get rid of Animal class completely and define what you need the way you need for all specific animals.

As for implementation, your approach is actually pretty good, using the fancy overloaded pattern. The only thing I can recommend you consider is to use a single generic lambda as a visitor with a bunch of if constexpr inside instead, as this might be easier to read in some circumstances. But this really depends and there is nothing bad in your approach.