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

Допустим, у меня есть следующий 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 имеют одинаковый код внутри фигурных скобок. Это дублирующий код и может вызвать проблемы с обслуживанием для длинных функций со сложной логикой .

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

Ответы [ 16 ]

163 голосов
/ 24 сентября 2008

Подробное объяснение см. В заголовке «Избегайте дублирования в const и функциях, отличных от const», на с. 23, в пункте 3 «Использовать const везде, где это возможно» в Эффективный C ++ , 3d ed. Scott Meyers, 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;
};

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

56 голосов
/ 24 сентября 2008

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

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
};

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

31 голосов
/ 28 мая 2013

Я думаю, что решение Скотта Мейерса может быть улучшено в 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 из-за шаблонов с переменными значениями.

20 голосов
/ 24 сентября 2008

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

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 & для константного экземпляра, поэтому он является закрытым. Приватные методы могут нарушать инварианты внешнего интерфейса (в этом случае желаемый инвариант - «объект const нельзя изменить с помощью ссылок, полученных через него, на объекты, которые он имеет -a»).

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

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

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

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

18 голосов
/ 18 ноября 2017

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());
}
16 голосов
/ 08 декабря 2013

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

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.

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

6 голосов
/ 13 января 2009

Вы также можете решить это с помощью шаблонов. Это решение несколько уродливо (но уродство скрыто в файле .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, добавленное во время тестирования.]

3 голосов
/ 24 сентября 2008

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

1 голос
/ 30 марта 2019

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

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

вот еще один дубль:

#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 счастливы (согласно Проводник компилятора ).

1 голос
/ 14 января 2017

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

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

};

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

...