Как удалить дублирование кода между похожими константными и неконстантными функциями-членами?

StackOverflow https://stackoverflow.com/questions/123758

Вопрос

Допустим, у меня есть следующее class X где я хочу вернуть доступ к внутреннему члену:

class Z
{
    // details
};

class X
{
    std::vector<Z> vecZ;

public:
    Z& Z(size_t index)
    {
        // massive amounts of code for validating index

        Z& ret = vecZ[index];

        // even more code for determining that the Z instance
        // at index is *exactly* the right sort of Z (a process
        // which involves calculating leap years in which
        // religious holidays fall on Tuesdays for
        // the next thousand years or so)

        return ret;
    }
    const Z& Z(size_t index) const
    {
        // identical to non-const X::Z(), except printed in
        // a lighter shade of gray since
        // we're running low on toner by this point
    }
};

Две функции-члена X::Z() и X::Z() const имеют идентичный код внутри фигурных скобок.Это дубликат кода и может вызвать проблемы с обслуживанием длинных функций со сложной логикой..

Есть ли способ избежать дублирования кода?

Это было полезно?

Решение 2

Да, дублирования кода можно избежать.Вам нужно использовать константную функцию-член, чтобы иметь логику, и чтобы неконстантная функция-член вызывала константную функцию-член и повторно приводила возвращаемое значение к неконстантной ссылке (или указателю, если функции возвращают указатель):

class X
{
   std::vector<Z> vecZ;

public:
   const Z& Z(size_t index) const
   {
      // same really-really-really long access 
      // and checking code as in OP
      // ...
      return vecZ[index];
   }

   Z& Z(size_t index)
   {
      // One line. One ugly, ugly line - but just one line!
      return const_cast<Z&>( static_cast<const X&>(*this).Z(index) );
   }

 #if 0 // A slightly less-ugly version
   Z& Z(size_t index)
   {
      // Two lines -- one cast. This is slightly less ugly but takes an extra line.
      const X& constMe = *this;
      return const_cast<Z&>( constMe.Z(index) );
   }
 #endif
};

ПРИМЕЧАНИЕ: Важно, что вы делаете НЕТ поместите логику в неконстантную функцию и заставьте константную функцию вызывать неконстантную функцию - это может привести к неопределенному поведению.Причина в том, что экземпляр константного класса преобразуется в непостоянный экземпляр.Неконстантная функция-член может случайно изменить класс, что, как утверждает стандарт C++, приведет к неопределенному поведению.

Другие советы

Подробное объяснение можно найти в разделе «Избегайте дублирования в const и не-const Функция-член» на стр.23, в п.3 «Использование const по возможности», в Эффективный С++, 3-е изд. Скотта Мейерса, ISBN-13:9780321334879.

alt text

Вот решение Мейерса (упрощенное):

struct C {
  const char & get() const {
    return c;
  }
  char & get() {
    return const_cast<char &>(static_cast<const C &>(*this).get());
  }
  char c;
};

Два приведения и вызов функции могут показаться некрасивыми, но это правильно.У Мейерса есть подробное объяснение, почему.

Я думаю, что решение Скотта Мейерса можно улучшить в C++11, используя вспомогательную функцию tempate.Это делает намерение намного более очевидным и может быть повторно использовано для многих других геттеров.

template <typename T>
struct NonConst {typedef T type;};
template <typename T>
struct NonConst<T const> {typedef T type;}; //by value
template <typename T>
struct NonConst<T const&> {typedef T& type;}; //by reference
template <typename T>
struct NonConst<T const*> {typedef T* type;}; //by pointer
template <typename T>
struct NonConst<T const&&> {typedef T&& type;}; //by rvalue-reference

template<typename TConstReturn, class TObj, typename... TArgs>
typename NonConst<TConstReturn>::type likeConstVersion(
   TObj const* obj,
   TConstReturn (TObj::* memFun)(TArgs...) const,
   TArgs&&... args) {
      return const_cast<typename NonConst<TConstReturn>::type>(
         (obj->*memFun)(std::forward<TArgs>(args)...));
}

Эту вспомогательную функцию можно использовать следующим образом.

struct T {
   int arr[100];

   int const& getElement(size_t i) const{
      return arr[i];
   }

   int& getElement(size_t i) {
      return likeConstVersion(this, &T::getElement, i);
   }
};

Первым аргументом всегда является указатель this.Второй — это указатель на вызываемую функцию-член.После этого можно передать произвольное количество дополнительных аргументов, чтобы их можно было передать в функцию.Для этого требуется C++11 из-за вариативных шаблонов.

Немного более многословно, чем Мейерс, но я мог бы сделать так:

class X {

    private:

    // This method MUST NOT be called except from boilerplate accessors.
    Z &_getZ(size_t index) const {
        return something;
    }

    // boilerplate accessors
    public:
    Z &getZ(size_t index)             { return _getZ(index); }
    const Z &getZ(size_t index) const { return _getZ(index); }
};

У закрытого метода есть нежелательное свойство: он возвращает неконстантное значение Z& для константного экземпляра, поэтому он является закрытым.Приватные методы могут нарушать инварианты внешнего интерфейса (в этом случае желаемым инвариантом является «константный объект не может быть изменен с помощью полученных через него ссылок на объекты, которые он имеет -a»).

Обратите внимание, что комментарии являются частью шаблона — интерфейс _getZ указывает, что его вызов никогда не является допустимым (за исключением методов доступа, очевидно):в любом случае в этом нет никакой мыслимой выгоды, потому что для ввода нужно будет на один символ больше, и это не приведет к уменьшению или ускорению кода.Вызов метода эквивалентен вызову одного из методов доступа с помощью const_cast, и вы тоже не захотите этого делать.Если вы беспокоитесь о том, чтобы ошибки стали очевидными (и это справедливая цель), назовите его const_cast_getZ вместо _getZ.

Кстати, я ценю решение Мейерса.У меня нет философских возражений против этого.Однако лично я предпочитаю немного контролируемого повторения и частный метод, который должен вызываться только в определенных строго контролируемых обстоятельствах, методу, который выглядит как линейный шум.Выберите свой яд и придерживайтесь его.

[Редактировать:Кевин справедливо заметил, что _getZ может захотеть вызвать дополнительный метод (скажем, GenerateZ), который специализируется на константах так же, как и getZ.В этом случае _getZ увидит константу Z& и должен будет преобразовать ее в const_cast перед возвратом.Это по-прежнему безопасно, поскольку шаблонный метод доступа контролирует все, но не так уж очевидно, что это безопасно.Более того, если вы сделаете это, а затем изменитеgenerateZ, чтобы он всегда возвращал const, вам также нужно будет изменить getZ, чтобы он всегда возвращал const, но компилятор не сообщит вам об этом.

Последний пункт о компиляторе также верен в отношении рекомендуемого шаблона Мейерса, но первый пункт о неочевидном const_cast — нет.Итак, в целом я думаю, что если _getZ окажется, что для возвращаемого значения потребуется const_cast, то этот шаблон потеряет большую часть своей ценности по сравнению с шаблоном Мейерса.Поскольку он также имеет недостатки по сравнению с Мейерсом, я думаю, что в этой ситуации я бы переключился на его.Рефакторинг от одного к другому прост — он не влияет на другой допустимый код в классе, поскольку только недопустимый код и шаблонный код вызывают _getZ.]

C++17 обновил лучший ответ на этот вопрос:

T const & f() const {
    return something_complicated();
}
T & f() {
    return const_cast<T &>(std::as_const(*this).f());
}

Это имеет преимущества:

  • Понятно, что происходит
  • Имеет минимальные накладные расходы на код — он умещается в одну строку.
  • Трудно ошибиться (можно только отбросить volatile случайно, но volatile это редкий признак)

Если вы хотите пройти полный путь вывода, то это можно сделать с помощью вспомогательной функции.

template<typename T>
constexpr T & as_mutable(T const & value) noexcept {
    return const_cast<T &>(value);
}
template<typename T>
void as_mutable(T const &&) = delete;

Теперь ты даже не можешь испортить volatile, и использование выглядит так

T & f() {
    return as_mutable(std::as_const(*this).f());
}

Хороший вопрос и хорошие ответы.У меня есть другое решение, которое не использует приведения:

class X {

private:

    std::vector<Z> v;

    template<typename InstanceType>
    static auto get(InstanceType& instance, std::size_t i) -> decltype(instance.get(i)) {
        // massive amounts of code for validating index
        // the instance variable has to be used to access class members
        return instance.v[i];
    }

public:

    const Z& get(std::size_t i) const {
        return get(*this, i);
    }

    Z& get(std::size_t i) {
        return get(*this, i);
    }

};

Однако у него есть уродство, заключающееся в том, что он требует статического члена и необходимости использования instance переменная внутри него.

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

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

.h-файл:

#include <vector>

class Z
{
    // details
};

class X
{
    std::vector<Z> vecZ;

public:
    const std::vector<Z>& GetVector() const { return vecZ; }
    std::vector<Z>& GetVector() { return vecZ; }

    Z& GetZ( size_t index );
    const Z& GetZ( size_t index ) const;
};

.cpp-файл:

#include "constnonconst.h"

template< class ParentPtr, class Child >
Child& GetZImpl( ParentPtr parent, size_t index )
{
    // ... massive amounts of code ...

    // Note you may only use methods of X here that are
    // available in both const and non-const varieties.

    Child& ret = parent->GetVector()[index];

    // ... even more code ...

    return ret;
}

Z& X::GetZ( size_t index )
{
    return GetZImpl< X*, Z >( this, index );
}

const Z& X::GetZ( size_t index ) const
{
    return GetZImpl< const X*, const Z >( this, index );
}

Главный недостаток, который я вижу, заключается в том, что, поскольку вся сложная реализация метода находится в глобальной функции, вам либо нужно получить члены X, используя общедоступные методы, такие как GetVector() выше (из которых всегда должен быть константная и неконстантная версия), или вы можете сделать эту функцию своей подругой.Но я не люблю друзей.

[Редактировать:удалено ненужное включение cstdio, добавленное во время тестирования.]

Как насчет того, чтобы перенести логику в частный метод и выполнять только действия «получить ссылку и вернуть» внутри геттеров?На самом деле, меня бы смутили статические и константные приведения внутри простой функции-получателя, и я бы посчитал это уродливым, за исключением крайне редких обстоятельств!

Является ли использование препроцессора обманом?

struct A {

    #define GETTER_CORE_CODE       \
    /* line 1 of getter code */    \
    /* line 2 of getter code */    \
    /* .....etc............. */    \
    /* line n of getter code */       

    // ^ NOTE: line continuation char '\' on all lines but the last

   B& get() {
        GETTER_CORE_CODE
   }

   const B& get() const {
        GETTER_CORE_CODE
   }

   #undef GETTER_CORE_CODE

};

Это не так сложно, как шаблоны или приведения, но делает ваше намерение («эти две функции должны быть идентичными») довольно явным.

Обычно функции-члены, для которых вам нужны константные и неконстантные версии, — это геттеры и сеттеры.В большинстве случаев они однострочные, поэтому дублирование кода не является проблемой.

Я сделал это для друга, который справедливо оправдал использование const_cast...не зная об этом, я бы, наверное, сделал что-то вроде этого (не очень элегантно):

#include <iostream>

class MyClass
{

public:

    int getI()
    {
        std::cout << "non-const getter" << std::endl;
        return privateGetI<MyClass, int>(*this);
    }

    const int getI() const
    {
        std::cout << "const getter" << std::endl;
        return privateGetI<const MyClass, const int>(*this);
    }

private:

    template <class C, typename T>
    static T privateGetI(C c)
    {
        //do my stuff
        return c._i;
    }

    int _i;
};

int main()
{
    const MyClass myConstClass = MyClass();
    myConstClass.getI();

    MyClass myNonConstClass;
    myNonConstClass.getI();

    return 0;
}

Я бы предложил шаблон статической функции частного помощника, например:

class X
{
    std::vector<Z> vecZ;

    // ReturnType is explicitly 'Z&' or 'const Z&'
    // ThisType is deduced to be 'X' or 'const X'
    template <typename ReturnType, typename ThisType>
    static ReturnType Z_impl(ThisType& self, size_t index)
    {
        // massive amounts of code for validating index
        ReturnType ret = self.vecZ[index];
        // even more code for determining, blah, blah...
        return ret;
    }

public:
    Z& Z(size_t index)
    {
        return Z_impl<Z&>(*this, index);
    }
    const Z& Z(size_t index) const
    {
        return Z_impl<const Z&>(*this, index);
    }
};

Для тех (как я), кто

  • использовать С++17
  • хочу добавить минимум шаблонного текста/повторение и
  • не против использовать макрос (в ожидании метаклассов...),

вот еще вариант:

#include <utility>
#include <type_traits>

template <typename T> struct NonConst;
template <typename T> struct NonConst<T const&> {using type = T&;};
template <typename T> struct NonConst<T const*> {using type = T*;};

#define NON_CONST(func)                                                     \
    template <typename... T>                                                \
    auto func(T&&... a) -> typename NonConst<decltype(func(a...))>::type {  \
        return const_cast<decltype(func(a...))>(                            \
            std::as_const(*this).func(std::forward<T>(a)...));              \
    }

По сути, это смесь ответов @Pait, @DavidStone и @sh1.В таблицу добавляется только одна дополнительная строка кода, которая просто называет функцию (но без дублирования аргументов или возвращаемого типа):

class X
{
    const Z& get(size_t index) const { ... }
    NON_CONST(get)
};

Примечание:gcc не может скомпилировать это до версии 8.1, clang-5 и выше, а также MSVC-19 счастливы (согласно проводник компилятора).

Меня удивляет, что существует так много разных ответов, но почти все они полагаются на тяжелую магию шаблонов.Шаблоны — это мощный инструмент, но иногда макросы превосходят их по лаконичности.Максимальная универсальность часто достигается за счет комбинирования того и другого.

я написал макрос FROM_CONST_OVERLOAD() который можно поместить в неконстантную функцию для вызова константной функции.

Пример использования:

class MyClass
{
private:
    std::vector<std::string> data = {"str", "x"};

public:
    // Works for references
    const std::string& GetRef(std::size_t index) const
    {
        return data[index];
    }

    std::string& GetRef(std::size_t index)
    {
        return FROM_CONST_OVERLOAD( GetRef(index) );
    }


    // Works for pointers
    const std::string* GetPtr(std::size_t index) const
    {
        return &data[index];
    }

    std::string* GetPtr(std::size_t index)
    {
        return FROM_CONST_OVERLOAD( GetPtr(index) );
    }
};

Простая и многоразовая реализация:

template <typename T>
T& WithoutConst(const T& ref)
{
    return const_cast<T&>(ref);
}

template <typename T>
T* WithoutConst(const T* ptr)
{
    return const_cast<T*>(ptr);
}

template <typename T>
const T* WithConst(T* ptr)
{
    return ptr;
}

#define FROM_CONST_OVERLOAD(FunctionCall) \
  WithoutConst(WithConst(this)->FunctionCall)

Объяснение:

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

return const_cast<Result&>( static_cast<const MyClass*>(this)->Method(args) );

Большую часть этого шаблона можно избежать, используя вывод типа.Первый, const_cast может быть инкапсулирован в WithoutConst(), который определяет тип своего аргумента и удаляет константный квалификатор.Во-вторых, аналогичный подход можно использовать и в WithConst() для const-квалификации this указатель, который позволяет вызвать перегруженный const метод.

Остальное — это простой макрос, который добавляет к вызову правильно указанный префикс. this-> и удаляет const из результата.Поскольку выражение, используемое в макросе, почти всегда представляет собой простой вызов функции с перенаправленными аргументами 1:1, недостатки макросов, такие как множественное вычисление, не проявляются.Многоточие и __VA_ARGS__ также можно использовать, но в этом нет необходимости, поскольку в круглых скобках встречаются запятые (как разделители аргументов).

Этот подход имеет несколько преимуществ:

  • Минимальный и естественный синтаксис — просто оберните вызов FROM_CONST_OVERLOAD( )
  • Никакой дополнительной функции-члена не требуется.
  • Совместимость с С++98.
  • Простая реализация, отсутствие шаблонного метапрограммирования и отсутствие зависимостей.
  • Расширяемый:могут быть добавлены другие константные отношения (например, const_iterator, std::shared_ptr<const T>, и т. д.).Для этого просто перегрузите WithoutConst() для соответствующих типов.

Ограничения:это решение оптимизировано для сценариев, в которых неконстантная перегрузка действует точно так же, как и константная перегрузка, поэтому аргументы можно пересылать 1:1.Если ваша логика отличается и вы не вызываете константную версию через this->Method(args), вы можете рассмотреть другие подходы.

Эта статья DDJ показывает способ использования специализации шаблона, который не требует использования const_cast.Хотя для такой простой функции это действительно не нужно.

boost::any_cast (в какой-то момент его больше нет) использует const_cast из константной версии, вызывая неконстантную версию, чтобы избежать дублирования.Однако вы не можете наложить константную семантику на неконстантную версию, поэтому вам придется очень осторожнее с этим.

В конце концов некоторое дублирование кода является хорошо, если два фрагмента находятся прямо друг над другом.

В дополнение к решению, предоставленному jwfearn и Кевином, вот соответствующее решение, когда функция возвращаетshared_ptr:

struct C {
  shared_ptr<const char> get() const {
    return c;
  }
  shared_ptr<char> get() {
    return const_pointer_cast<char>(static_cast<const C &>(*this).get());
  }
  shared_ptr<char> c;
};

Не нашел того, что искал, поэтому накатил парочку своих...

Этот вариант немного многословен, но его преимущество заключается в одновременной обработке множества перегруженных методов с одинаковым именем (и типом возвращаемого значения):

struct C {
  int x[10];

  int const* getp() const { return x; }
  int const* getp(int i) const { return &x[i]; }
  int const* getp(int* p) const { return &x[*p]; }

  int const& getr() const { return x[0]; }
  int const& getr(int i) const { return x[i]; }
  int const& getr(int* p) const { return x[*p]; }

  template<typename... Ts>
  auto* getp(Ts... args) {
    auto const* p = this;
    return const_cast<int*>(p->getp(args...));
  }

  template<typename... Ts>
  auto& getr(Ts... args) {
    auto const* p = this;
    return const_cast<int&>(p->getr(args...));
  }
};

Если у вас есть только один const метода для каждого имени, но все же существует множество методов для дублирования, то вы можете предпочесть это:

  template<typename T, typename... Ts>
  auto* pwrap(T const* (C::*f)(Ts...) const, Ts... args) {
    return const_cast<T*>((this->*f)(args...));
  }

  int* getp_i(int i) { return pwrap(&C::getp_i, i); }
  int* getp_p(int* p) { return pwrap(&C::getp_p, p); }

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

  template<typename... Ts>
  auto* getp(Ts... args) { return pwrap<int, Ts...>(&C::getp, args...); }

Но отсылайте аргументы к const метод не сопоставляется с явно выраженными аргументами шаблона, и он ломается. Не знаю почему.Вот почему.

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top