У меня есть проект среднего размера, в котором используется иерархия классов, которая по сути сводится к следующему:

class Mach
{
 public:
    virtual ~Mach();
    //...
    virtual void common_action() =0;
    //... A lot of common actions
    //...
};

class MachStrato : public Mach
{
 public: //...
    // ...Implementation of the common interface
    virtual void common_action() override { /* ... */ }
    //...

    // ...But also some actions specific to this machine
    void specific_strato_action();
};

class MachFloat : public Mach
{
 public: //...
    // ...Implementation of the common interface
    virtual void common_action() override { /* ... */ }
    //...

    // ...But also some actions specific to this machine
    void specific_float_action();
};

Код использует только один экземпляр одного из этих классов, созданный на ранних этапах в зависимости от внешних аргументов. В коде используются преимущественно общие Mach интерфейс, но иногда приходится проверять фактический тип машины, чтобы получить доступ к ее конкретным действиям. Во избежание повторения dynamic_cast в каждом из этих случаев, но чтобы сделать это один раз, я решил обернуть их в класс, который, помимо управления памятью, предоставляет мне информацию о фактическом типе объекта; своего рода настраиваемый интеллектуальный указатель с учетом иерархии:

// Create, take ownership and invoke the machine instance
class MachInstance
{
 public:
    MachInstance() noexcept :
        i_mach(nullptr),
        i_stratomach(nullptr),
        i_floatmach(nullptr) {}

    MachInstance(Mach* const m) noexcept :
        i_mach(m), 
        i_stratomach(dynamic_cast<MachStrato*>(m)), 
        i_floatmach(dynamic_cast<MachFloat*>(m)) {}

    ~MachInstance() { clear(); }

    // Take ownership of an existing machine (!contract: on the heap!)
    MachInstance& operator=(Mach* const m)
       {
        clear();
        i_mach = m;
        i_stratomach = dynamic_cast<MachStrato*>(i_mach);
        i_floatmach = dynamic_cast<MachFloat*>(i_mach);
        return *this;
       }

    void create( const std::string& arg )
       {
        clear();
        if( arg.find("float")!=std::string::npos )
           {
            i_mach = new MachFloat(arg);
           }
        else
           {
            i_mach = new MachStrato(arg);
           }
        i_stratomach = dynamic_cast<MachStrato*>(i_mach);
        i_floatmach = dynamic_cast<MachFloat*>(i_mach);
       }

    void clear()
       {
        if(i_mach)
           {
            delete i_mach;
            i_mach = nullptr;
           }
        i_stratomach = nullptr;
        i_floatmach = nullptr;
       }
 
    Mach* release()
       {
        Mach* m = i_mach;
        i_mach = nullptr;
        clear();
        return m;
       }

    operator bool() const { return i_mach!=nullptr; }
    Mach* ptr() const { return i_mach; }
    Mach* operator->() const { return i_mach; }
    
    // Unchecked
    MachStrato* as_strato() const { return i_stratomach; }
    MachFloat* as_float() const { return i_floatmach; }
    
private:
    Mach* i_mach;
    MachStrato* i_stratomach;
    MachFloat* i_floatmach;
    //...Other types may be -laboriously- added
    // Note: Do not use a union, a specialization could be nullptr
    //union  {
    //        Mach* i_mach;
    //        MachStrato* i_stratomach;
    //        MachFloat* i_floatmach;
    //       };

    MachInstance(const MachInstance&)=delete; // Prevent copy
    MachInstance& operator=(const MachInstance&)=delete; // Prevent copy assignment
};

Использование выглядит так:

MachInstance mach;
// Instantiate machine basing on external arguments
mach.create("strato");
// Accessing common interface
if(mach) mach->common_action();
// Accessing specific interface
if( user_wants_strato_action )
   {
    if( !mach.as_strato() ) throw std::runtime_error("Need a strato machine for this!");
    mach.as_strato()->specific_strato_action();
   }
if( user_wants_float_action )
   {
    if( !mach.as_float() ) throw std::runtime_error("Need a float machine for this!");
    mach.as_float()->specific_float_action();
   }

Мне очень нравится использование MachInstance, но меня беспокоит то, что он не очень масштабируемый; если типы машин n, это содержит n+1 разные указатели. Мне было интересно, что вы думаете об этой стратегии и подойдете ли вы к этой проблеме по-другому.

Обзор дизайна / кода

В первом разделе нечего проверять … потому что там просто нет кода. Единственное, что нужно сделать:

virtual void common_action() override { /* ... */ }

Использовать virtual ИЛИ ЖЕ override… не оба. (На самом деле, правило: «используйте только один из virtual, override, или же final… Но вы почти никогда не должны использовать final.)

Так что единственное, что нужно проверить, — это MachInstance класс. Но вы задали несколько конкретных вопросов, поэтому я просто включу большую часть обзора в ответы.

Здесь я просто сосредоточусь на некоторых вещах высокого уровня.

Я не думаю, что дизайн MachInstance вообще имеет смысл. Я не имею в виду все указатели в классе, я вообще имею в виду саму идею класса. Вы в основном переписали std::unique_ptr<Mach>. Шутки в сторону. Помимо create() функция … к которой я вернусь … это просто std::unique_ptr<Mach>. Вы можете отбросить весь класс, и ваш код будет выглядеть так:

auto create(std::string const& arg) -> std::unique_ptr<Mach>
{
    if (arg.contains("float"))
        return std::unique_ptr<Mach>{new MachFloat(arg)};
    else
        return std::unique_ptr<Mach>{new MachStrato(arg)};
}

auto mach = create("strato");

// Accessing common interface
if(mach) mach->common_action();

// Accessing specific interface
if (user_wants_strato_action)
{
    if (auto const p = dynamic_cast<MachStrato*>(mach.get()); p)
        p->specific_strato_action();
    else
        throw std::runtime_error{"Need a strato machine for this!"};
}
if (user_wants_float_action)
{
    if (auto const p = dynamic_cast<MachFloat*>(mach.get()); p)
        p->specific_float_action();
    else
        throw std::runtime_error{"Need a float machine for this!"};
}

Интерфейс почти такой же.

Вы заметите, что я вытащил create() функции и сделали ее бесплатной функцией. Даже если ты оставишь MachInstance, то create() функция — плохая идея. Это двухэтапная инициализация, и ее не одобряли, по крайней мере, с конца 1990-х годов.

Вся суть конструкторов в том, чтобы… янно… сконструировать объект. Ваш конструктор оставляет объект в полузавершенном, незавершенном состоянии, которое вы затем должны использовать create() на самом деле завершить. Это фактически добавляет ваш сложности для класса, потому что вы должны бороться с возможностью того, что объект находится в полузавершенном, незавершенном состоянии. Если вы просто использовали конструктор, чтобы… янно… построить объект… полностью… Тогда вам не потребуется особая сложность класса.

В любом случае, в достойном дизайне единственная функция, которую вам нужно изменить при добавлении нового типа машины, — это create() (Я покажу как позже). Я бы предложил либо сделать его бесплатной функцией, либо, возможно, статической функцией-членом Mach, так что ты бы написал auto mach = Mach::create("strato"). (Логичное место для размещения create()в этом случае находится в файле реализации, поэтому, когда вы добавляете новый тип машины, вам нужно только перекомпилировать этот один файл, а затем повторно связать его.)

Я думаю, что это все, что касается общего обзора дизайна. Итак, вот несколько предложений по решению вашей проблемы (с дополнительными подробностями обзора, разбросанными повсюду).

Вариант 1: отказ от наследования и виртуальных функций

В прежние времена новичков в C ++ учили, что способ делать сложные вещи в C ++ — это создавать базовый класс, выводить из него конкретные классы реализации и использовать виртуальные функции для получения полиморфного поведения. Все изменилось, и сегодня, когда я преподаю C ++ новичков, я учу, что использование динамического полиморфизма должно быть ПОСЛЕДНИЙ прибегать к использованию только после того, как вы рассмотрели все возможные варианты дизайна и отклонили их все.

Итак, позвольте мне предложить другой подход к проблеме.

1. Не создавайте базовый класс, если он на самом деле помогает. Другими словами, не создавайте базовый класс только потому, что вам нужен базовый класс для полиморфной динамической отправки. В вашем случае, по-видимому, существует масса общего поведения… или есть? Недостаточно кода, чтобы сказать. Если ваш виртуальный common_action() функции на самом деле не являются общими — например, каждый класс отличается — тогда нет смысла помещать их в базовый класс. Но если все они имеют общее поведение, то обязательно положите это в основу.

2. если ты делать создать базовый класс, не создавая никаких виртуальных функций (включая деструктор). Это просто приводит к генерации vtable, что является пустой тратой времени.

3. Создайте свои классы для каждой «вещи»: «машин» в вашем случае. Им не нужна общая база. Им даже не нужен общий интерфейс. (Хотя, когда общий интерфейс имеет смысл, тогда, конечно, он имеет смысл.)

4. Используйте тип суммы для сбора ваших классов. Стандартный тип суммы: std::variant.

5. Используйте посещение, чтобы получить полиморфное поведение.

Вот как это может выглядеть:

class MachStrato
{
public:
    constexpr void common_action();

    constexpr void specific_strato_action();

    // etc.
};

class MachFloat
{
public:
    constexpr void common_action();

    constexpr void specific_float_action();

    // etc.
};

class Mach
{
public:
    constexpr explicit Mach(std::string_view s) :
        _mach{_create(s)}
    {}

    // the shared interface
    constexpr void common_action()
    {
        std::visit([](auto&& m) { m.common_action(); }, _mach);

        // if a machine doesn't implement common_action(), you'll get a
        // compile-time error
    }

    // if you want a specific machine type, you can get it like this
    template <typename T> constexpr auto as()      &  -> T&        { return std::get<T>(_mach); }
    template <typename T> constexpr auto as() const&  -> T const&  { return std::get<T>(_mach); }
    template <typename T> constexpr auto as()      && -> T&&       { return std::get<T>(std::move(_mach)); }
    template <typename T> constexpr auto as() const&& -> T const&& { return std::get<T>(std::move(_mach)); }

    // to query the type
    template <typename T> constexpr auto is() const noexcept { return std::holds_alternative<T>(_mach); }

private:
    // there are two places you have to describe a new machine

    // #1
    using machine_types = std::variant<
        MachStrato,
        MachFloat,
        // ... add a new machine here
    >

    // #2
    static constexpr auto _create(std::string_view s)
    {
        // whatever the logic to determine which machine to create from a
        // string, put it here

        if (name.contains("float"))
            return MachFloat{s};
        else // the default?
            return MachStrato{s};
    }

    machine_types _mach;
};

// usage:
auto mach = Mach{"strato"};

mach.common_action();

// Accessing specific interface
if (user_wants_strato_action)
{
    mach.as<MachStrato>.specific_strato_action(); // will throw std::bad_variant_access if wrong type
}
if (user_wants_float_action)
{
    
    if (not mach.is<MachFloat>())
        throw std::runtime_error("Need a float machine for this!");
    mach.as<MachFloat>.specific_float_action();
}

Где возможно, это можно оптимизировать много лучше, чем версия с динамической отправкой. И это может быть constexpr.

Вариант 2: используйте шаблоны (и умные указатели!)

Итак, для этого варианта оставьте свой Mach и его производные классы в точности как есть. MachInstance это то место, где произойдет вся магия.

Во-первых, прекратите вручную управлять памятью. Это 2021 год. Сейчас мы используем умные указатели.

Хитрость здесь в том, чтобы взять все эти as_strato(), as_float() функций и используйте вместо этого один шаблон.

class MachInstance
{
public:
    // is it really necessary to be able to create an "empty" machine?
    constexpr MachInstance() noexcept  = default;

    explicit MachInstance(std::unique_ptr<Mach> m) noexcept :
        _mach{std::move(m)}
    {}

    // no need to write destructor
    //
    // class is automatically non-copyable
    // class is automatically no-fail movable

    // Take ownership of an existing machine (note, *must* be on the heap, because smart pointer)
    auto operator=(std::unique_ptr<Mach> m) noexcept -> MachInstance&
    {
        _mach = std::move(m);
        return *this;
    }

    auto create(std::string const& arg)
    {
        // clear(); <- don't clear before constructing the new data; what if it fails?

        if (arg.contains("float"))
        {
            _mach.reset(new MachFloat(arg));
        }
        else
        {
            _mach.reset(new MachStrato(arg));
        }
    }

    // are these necessary? don't add stuff to the interface you don't need
    auto clear() noexcept { _mach.reset(); }
    auto release() noexcept { return _mach.release(); }
    explicit operator bool() const noexcept { return bool(_mach); }

    // in the standard pointer interface, this is spelled `get()`
    Mach* ptr() const { return i_mach; }

    auto operator->() const { return _mach; }

    // might as well add this
    // if you don't allow an "empty" state, then this could be noexcept
    auto operator*()       -> Mach      & { return *_mach; }
    auto operator*() const -> Mach const& { return *_mach; }

    template <typename T> constexpr auto as()       -> T*        { return dynamic_cast<T      *>(_mach.get()); }
    template <typename T> constexpr auto as() const -> T const*  { return dynamic_cast<T const*>(_mach.get()); }

private:
    std::unique_ptr<Mach> _mach;
};

Примечание: это весь MachInstance класс. Ничего не пропало (что я могу придумать), и на самом деле, я думаю, что многие из этих функций могут исчезнуть. Меньше — больше. Чем больше дерьма вы положите в класс, тем больше шансов, что материал сломается. (И, конечно же, еще надо тестировать и т. Д.)

И использование:

MachInstance mach;
// Instantiate machine basing on external arguments
mach.create("strato");
// Accessing common interface
if(mach) mach->common_action();
// Accessing specific interface
if( user_wants_strato_action )
   {
    if( !mach.as<MachStrato>() ) throw std::runtime_error("Need a strato machine for this!");
    mach.as<MachStrato>()->specific_strato_action();
   }
if( user_wants_float_action )
   {
    if( !mach.as<MachFloat>() ) throw std::runtime_error("Need a float machine for this!");
    mach.as<MachFloat>()->specific_float_action();
   }

Обратите внимание, что единственное, что я изменил в этом интерфейсе, — это as_strato() а также as_float() вызовы функций. Все остальное нетронуто.