Comment puis-je supprimer la duplication de code entre des fonctions similaires de membre const et non-const?

disons que j'ai le suivant class X où je veux rendre l'accès à un membre interne:

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

les deux fonctions X::Z() et X::Z() const ont un code identique à l'intérieur des bretelles. Il s'agit du code double et peut causer des problèmes de maintenance pour les fonctions longues avec la logique complexe .

y a-t-il un moyen d'éviter cette duplication de code?

187
demandé sur sharptooth 2008-09-24 00:47:11

15 réponses

pour une explication détaillée, voir la rubrique "éviter la Duplication dans const et non - const , à la p. 23, in Item 3 "Use const whenever possible," in Effective C++ , 3d ed by Scott Meyers, ISBN-13: 9780321334879.

alt text

Voici la solution de Meyers (simplifiée):

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

les deux casts et l'appel de fonction peuvent être moches mais c'est correct. Meyers a une explication complète.

153
répondu jwfearn 2017-06-01 01:24:08

Oui, il est possible d'éviter la duplication du code. Vous devez utiliser la fonction de membre const pour avoir la logique et avoir la fonction de membre non-const appeler la fonction de membre const et rejouer la valeur de retour à une référence non-const (ou pointeur si les fonctions renvoie un pointeur):

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

NOTE: Il est important que vous le faites PAS mettre de la logique dans le non-const fonction et la const-fonction appelez la fonction non-const -- cela peut donner un comportement non défini. La raison en est qu'une instance de classe constante est coulée comme une instance non constante. La fonction de membre non-const peut accidentellement modifier la classe, ce que les États standards C++ vont conduire à un comportement non défini.

52
répondu Kevin 2011-09-16 22:29:54

je pense que la solution de Scott Meyers peut être améliorée en C++11 en utilisant une fonction d'aide temporaire. Cela rend l'intention beaucoup plus évidente et peut être réutilisée pour de nombreux autres getters.

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

Cette fonction d'aide peut être utilisée de la manière suivante.

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

le premier argument est toujours le pointeur. Le second est le pointeur vers la fonction membre à appeler. Après qu'une quantité arbitraire d'arguments supplémentaires peuvent être transmises de sorte qu'ils puissent être transmis à la fonction. Cela nécessite du C++11 à cause des modèles variadiques.

29
répondu Pait 2017-05-19 10:08:06

un peu plus verbeux que Meyers, mais je pourrais le faire:

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

La méthode privée a la propriété indésirable qu'il renvoie d'un non-const Z& pour un const exemple, qui est pourquoi il est privé. Les méthodes privées peuvent casser les invariants de l'interface externe (dans ce cas l'invariant désiré est "un objet const ne peut pas être modifié via des références obtenues à travers lui aux objets qu'il a-a").

notez que les commentaires font partie du l'interface de pattern - _getZ spécifie qu'il n'est jamais valide de l'appeler (en dehors des accesseurs, évidemment): il n'y a aucun avantage concevable à le faire de toute façon, parce que c'est 1 caractère de plus à taper et ne résultera pas en un code plus petit ou plus rapide. Appeler la méthode est équivalent à appeler un des accesseurs avec un const_cast, et vous ne voudriez pas faire cela non plus. Si vous craignez de faire des erreurs évidentes (et c'est un objectif raisonnable), appelez-le const_cast_getZ au lieu de _getZ.

au fait, J'apprécie la solution de Meyers. Je n'y vois aucune objection philosophique. Personnellement, cependant, je préfère un petit peu de répétition contrôlée, et une méthode privée qui ne doit être appelée que dans certaines circonstances étroitement contrôlées, plutôt qu'une méthode qui ressemble à du bruit de ligne. Choisis ton poison et reste avec.

[Edit: Kevin a fait remarquer à juste titre que _getZ pourrait appeler une autre méthode (disons generateZ) qui est const-spécialisé de la même façon que getZ. Dans ce cas, _getZ verrait une const Z& et devrait La calculer avant le retour. C'est encore sûr, puisque l'accessor boilerplate règle tout, mais ce n'est pas évident que ce soit sûr. De plus, si vous faites cela et puis changez generateZ pour toujours retourner const, alors vous devez aussi changer getZ pour toujours retourner const, mais le compilateur ne vous dira pas que vous le faites.

ce dernier point sur le compilateur est également vrai du modèle recommandé de Meyers, mais le premier point sur une constance non-évidente n'est pas. Donc dans l'ensemble, je pense que si _getZ s'avère avoir besoin d'un const_cast pour sa valeur de retour, alors ce modèle perd beaucoup de sa valeur par rapport à Meyers. Comme il souffre aussi des désavantages par rapport à Meyers, je pense que je changerais à la sienne dans cette situation. Refactoring de l'un à l'autre est facile-il n'affecte pas les autres code valide dans la classe, puisque seul le code non valide et le boilerplate appelle Getz.]

18
répondu Steve Jessop 2008-09-24 13:13:32

question Sympa et agréable réponses. J'ai une autre solution, qui n'utilise pas de moulages:

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

};

cependant, il a la laideur d'exiger un élément statique et la nécessité d'utiliser la variable instance à l'intérieur de lui.

Je n'ai pas considéré toutes les implications (négatives) possibles de cette solution. S'il vous plaît laissez-moi savoir si tout.

15
répondu gd1 2013-12-19 16:54:36

C++17 a mis à jour la meilleure réponse à cette question:

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

cela a les avantages qu'il:

  • Est évident que ce qui se passe
  • a un code minimal au-dessus -- il s'inscrit dans une seule ligne
  • est difficile de se tromper (ne peut rejeter volatile que par accident, mais volatile est un qualificatif rare)

Si vous voulez faire le plein déduction parcours qui peut être accompli en ayant une fonction d'assistance

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;

Maintenant vous ne pouvez même pas gâcher volatile , et l'utilisation ressemble à

T & f() {
    return as_mutable(std::as_const(*this).f());
}
13
répondu David Stone 2018-07-08 16:16:42

vous pouvez également résoudre ce problème avec des modèles. Cette solution est légèrement laide (mais la laideur est cachée dans le .cpp file), mais il fournit la vérification du compilateur de Constance, et aucune duplication de code.

.h fichier:

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

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

le principal inconvénient que je peux voir est que parce que toute la mise en œuvre complexe de la méthode est dans une fonction globale, vous avez soit besoin de mettre la main sur le les membres de X utilisant des méthodes publiques comme GetVector () ci-dessus (dont il y a toujours besoin d'une version const et non-const) ou vous pouvez faire de cette fonction un ami. Mais je n'aime pas les amis.

[Edit: suppression de l'inutile de l'inclure des cstdio ajouté pendant le test.]

6
répondu Andy Balaam 2009-01-13 16:33:38

pourquoi ne pas déplacer la logique dans une méthode privée, et ne faire que le truc "obtenir la référence et retourner" à l'intérieur des getters? En fait, je serais assez confus au sujet de la statique et const jette à l'intérieur d'une fonction getter simple, et je considérerais cela laid sauf dans des circonstances extrêmement rares!

3
répondu MP24 2008-09-23 22:05:02

j'ai fait cela pour un ami qui justifiait légitimement l'utilisation de const_cast ... sans le savoir j'aurais probablement fait quelque chose comme ça (pas très élégant) :

#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;
}
1
répondu matovitch 2015-06-16 13:13:06

c'Est de la triche d'utiliser le préprocesseur?

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

};

ce n'est pas aussi fantaisiste que des modèles ou des moulages, mais cela rend votre intention ("ces deux fonctions doivent être identiques") assez explicite.

1
répondu user1476176 2017-01-14 02:33:05

typiquement, les fonctions de membre pour lesquelles vous avez besoin des versions const et non-const sont getters et setters. La plupart du temps, ils ne sont qu'une couche, donc la duplication de code n'est pas un problème.

0
répondu Dima 2008-09-23 21:11:44

pour ajouter à la solution fournie par jwfearn et kevin, voici la solution correspondante lorsque la fonction retourne 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;
};
0
répondu Christer Swahn 2014-11-20 14:06:10

je suggérerais un modèle de fonction statique d'Assistant privé, comme ceci:

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);
    }
};
0
répondu dats 2015-10-07 14:25:20

N'a pas trouvé ce que je cherchais, donc j'en ai roulé quelques-uns...

celui-ci est un peu verbeux, mais a l'avantage de manipuler beaucoup de méthodes surchargées du même nom (et le type de retour) tout à la fois:

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

si vous avez seulement une méthode const par nom, mais encore beaucoup de méthodes à dupliquer, alors vous pourriez préférer ceci:

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

Malheureusement, cela se décompose dès que vous commencez à surcharger le nom (la liste d'arguments de l'argument pointeur de fonction semble être non résolue à ce point, de sorte qu'il ne peut pas trouver une correspondance pour l'argument de fonction). Bien que vous puissiez aussi trouver un modèle pour vous en sortir:

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

mais les arguments de référence à la méthode const ne correspondent pas avec les arguments apparemment De by-value au Modèle et il casse. Je ne sais pas pourquoi. voilà pourquoi .

0
répondu sh1 2017-05-23 12:26:33

cet article de DDJ montre une façon d'utiliser la spécialisation de modèle qui ne vous oblige pas à utiliser const_cast. Pour une fonction simple, il n'est vraiment pas nécessaire.

boost:: any_cast (at one point, it doesn't any more) utilise un const_cast de la version const appelant la version non-const pour éviter la duplication. Vous ne pouvez pas imposer la sémantique de const sur la version non-const, donc vous devez être très prudent avec que.

en fin de compte une certaine duplication de code est d'accord tant que les deux bribes sont directement au-dessus de l'autre.

-1
répondu Greg Rogers 2008-09-23 21:03:38