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


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

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

15 187

15 ответов:

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

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

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

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++ приведет к неопределенному поведению.

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

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

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

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

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

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

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 переменной внутри него.

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

в 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());
}

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

.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 () выше (из которых всегда должна быть версия const и non-const), или вы можете сделать эту функцию другом. Но я не люблю друзей.

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

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

Я сделал это для друга, который оправдано использование 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;
}

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

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 и non-const, являются геттерами и сеттерами. В большинстве случаев они являются однострочными, поэтому дублирование кода не является проблемой.

чтобы добавить к решению jwfearn и kevin, вот соответствующее решение, когда функция возвращает 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;
};

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

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

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

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

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 метод не соответствует по-видимому по значению аргументов шаблона, и он ломается. не знаю, почему.вот почему.

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

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

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