Comment supprimer la duplication de code entre des fonctions membres const et non const similaires?

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

Question

Supposons que je dispose des éléments class X suivants, dans lesquels je souhaite 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 membres X::Z() et X::Z() const ont un code identique à l'intérieur des accolades. Ce code est en double et peut entraîner des problèmes de maintenance pour les fonctions longues à logique complexe .

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

Était-ce utile?

La solution 2

Oui, il est possible d'éviter la duplication de code. Vous devez utiliser la fonction membre const pour avoir la logique et demander à la fonction membre non const d'appeler la fonction membre const et de rediffuser la valeur de retour en une référence non const (ou un pointeur si la fonction retourne 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
};

REMARQUE: il est important de NOT mettre la logique dans la fonction non-const et faire en sorte que la fonction const appelle la fonction non-const. peut entraîner un comportement indéfini. La raison en est qu'une instance de classe constante est convertie en instance non constante. La fonction membre non-const peut modifier accidentellement la classe, ce que les états standard C ++ entraîneront un comportement indéfini.

Autres conseils

Pour une explication détaillée, reportez-vous à la rubrique & "Eviter la duplication dans const et la fonction membre <=>, &"; dans. 23, élément 3 & "Utilisez <=> chaque fois que possible, &"; dans Effective C ++ , troisième édition de 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 lancers et appel de fonction peuvent être laids mais c’est correct. Meyers a une explication approfondie pourquoi.

Je pense que la solution de Scott Meyers peut être améliorée en C ++ 11 en utilisant une fonction d'assistance Tempate. Cela rend l'intention beaucoup plus évidente et peut être réutilisé par de nombreux autres utilisateurs. 

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'assistance 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 this. Le second est le pointeur sur la fonction membre à appeler. Après cela, une quantité arbitraire d'arguments supplémentaires peut être transmise pour pouvoir être transmise à la fonction. Cela nécessite C ++ 11 à cause des modèles variadiques.

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 private a la propriété indésirable de retourner un non-const Z & amp; pour une instance const, c'est pourquoi il est privé. Les méthodes privées peuvent rompre les invariants de l’interface externe (dans ce cas, l’invariant souhaité est & "; Un objet const ne peut pas être modifié via les références obtenues par ce biais aux objets qu’il a-un &";).

Notez que les commentaires font partie du modèle - l'interface de _getZ spécifie qu'il n'est jamais valide de l'appeler (hormis les accesseurs, évidemment): il n'y a aucun avantage concevable à le faire de toute façon, car il faut 1 caractère de plus à taper et ne donnera pas un code plus petit ou plus rapide. Appeler la méthode équivaut à appeler l'un des accesseurs avec un const_cast, et vous ne voudriez pas le faire non plus. Si vous craignez de faire des erreurs évidentes (et que c'est un objectif juste), appelez-le alors const_cast_getZ au lieu de _getZ.

Au fait, j’apprécie la solution de Meyers. Je n'ai aucune objection philosophique à cela. 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, par rapport à une méthode qui ressemble à du bruit de ligne. Choisissez votre poison et respectez-le.

[Edit: Kevin a fait remarquer à juste titre que _getZ pourrait appeler une autre méthode (par exemple generateZ) spécialisée dans const de la même manière que getZ. Dans ce cas, _getZ verrait un const Z & Amp; et doivent le const_cast avant de revenir. Cela reste sûr, car l’accessoire passe-partout surveille tout, mais il n’est pas particulièrement évident que ce soit sans danger. De plus, si vous faites cela et que vous modifiez plus tard generateZ pour qu'il renvoie toujours const, vous devez également modifier getZ pour toujours renvoyer const, mais le compilateur ne vous le dira pas.

Ce dernier point sur le compilateur est également vrai du modèle recommandé par Meyers, mais le premier point sur un const_cast non évident ne l'est pas. Donc dans l'ensemble, je pense que si _getZ a besoin d'un const_cast pour sa valeur de retour, alors ce modèle perd beaucoup de sa valeur par rapport à celui de Meyers. Comme il souffre également de désavantages par rapport à Meyers, je pense que je changerais pour le sien dans cette situation. Il est facile de refactoriser de l'un à l'autre - cela n'affecte aucun autre code valide de la classe, car seul le code non valide et le standard appelle _getZ.]

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 présente les avantages suivants:

  • Ce qui se passe est évident
  • La surcharge de code est minimale. Elle s’inscrit dans une seule ligne
  • Difficile de se tromper (vous ne pouvez rejeter que volatile par accident, mais <=> est un qualificatif rare)

Si vous souhaitez utiliser la voie de déduction complète, vous pouvez le faire en utilisant 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 plus vous tromper <=>, et l'utilisation ressemble à 

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

Belle question et bonnes réponses. J'ai une autre solution, qui n'utilise pas d'incantation: 

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 de requérir un membre statique et la nécessité d'utiliser la variable instance qu'il contient.

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

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 fichier .cpp), mais elle fournit une vérification de la constance par le compilateur et aucune duplication de code.

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

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 constate est que, comme toute la mise en œuvre complexe de la méthode est une fonction globale, vous devez soit contacter les membres de X à l’aide de méthodes publiques telles que GetVector () ci-dessus (dont il faut toujours être une version constante et non constante) ou vous pourriez faire de cette fonction un ami. Mais je n'aime pas les amis.

[Edition: suppression de l'inclusion inutile de cstdio ajoutée lors des tests.]

Que diriez-vous de déplacer la logique dans une méthode privée et de ne faire que & "obtenir la référence et retourner &"; des trucs à l'intérieur des getters? En fait, je serais assez confus à propos des charges statiques et constantes à l'intérieur d'une simple fonction de lecture, et je considérerais cela comme laid, sauf dans des cas extrêmement rares!

Est-il tricher 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 sophistiqué que les modèles ou les conversions, mais cela rend votre intention (& "; ces deux fonctions doivent être identiques &";) plutôt explicites.

En règle générale, les fonctions membres pour lesquelles vous avez besoin de versions const et non const sont des accesseurs et des passeurs. La plupart du temps, ils ne font qu'un. La duplication de code n'est donc pas un problème.

Je l'ai fait pour un ami qui justifiait à juste titre l'utilisation de const_cast ... sans le savoir, j'aurais probablement fait quelque chose comme ça (pas vraiment é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;
}

Je suggérerais un modèle de fonction statique d'assistance privée, 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);
    }
};

Pour ceux (comme moi) qui

  • utiliser c ++ 17
  • souhaitez ajouter la quantité minimale de passe-partout / répétition et
  • cela ne vous dérange pas d'utiliser makros (en attendant les méta-classes ...),

voici une autre prise: 

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

C’est à la base un mélange des réponses de @Pait, @DavidStone et @ sh1. Cela ajoute au tableau que vous vous en sortez avec une seule ligne de code supplémentaire qui nomme simplement la fonction (mais pas de duplication de type argument ou return): 

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

Remarque: gcc ne parvient pas à la compiler avant les versions 8.1, clang-5 et ultérieures ainsi que MSVC-19 sont satisfaits (selon l'explorateur du compilateur ).

C’est surprenant pour moi qu’il y ait tant de réponses différentes, mais presque toutes reposent sur une lourde magie. Les modèles sont puissants, mais parfois les macros les battent avec concision. La polyvalence maximale est souvent obtenue en combinant les deux.

J'ai écrit une macro FROM_CONST_OVERLOAD() qui peut être placée dans la fonction non const pour appeler la fonction const.

Exemple d'utilisation: 

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

Implémentation simple et réutilisable: 

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)

Explication:

Comme indiqué dans de nombreuses réponses, le schéma typique pour éviter la duplication de code dans une fonction membre non-const est le suivant: 

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

On peut éviter beaucoup de ce passe-partout en utilisant l’inférence de type. Premièrement, const_cast peut être encapsulé dans WithoutConst(), qui déduit le type de son argument et supprime le qualificatif const. Deuxièmement, une approche similaire peut être utilisée dans WithConst() pour qualifier en const le pointeur this, ce qui permet d'appeler la méthode const-overloaded.

Le reste est une simple macro qui préfixe l'appel avec le qualificateur correct this-> et supprime const du résultat. Étant donné que l’expression utilisée dans la macro est presque toujours un simple appel de fonction avec des arguments transférés 1: 1, les inconvénients des macros telles que l’évaluation multiple ne se manifestent pas. Les points de suspension et __VA_ARGS__ peuvent également être utilisés, mais ne sont pas nécessaires, car des virgules (en tant que séparateurs d'arguments) apparaissent entre parenthèses.

Cette approche présente plusieurs avantages:

  • Syntaxe minimale et naturelle - enveloppez simplement l'appel dans FROM_CONST_OVERLOAD( )
  • Aucune fonction de membre supplémentaire requise
  • Compatible avec C ++ 98
  • Implémentation simple, pas de métaprogrammation de modèles et aucune dépendance
  • Extensible: d'autres relations const peuvent être ajoutées (comme const_iterator, std::shared_ptr<const T>, etc.). Pour cela, il suffit de surcharger this->Method(args) les types correspondants.

Limitations: cette solution est optimisée pour les scénarios dans lesquels la surcharge non const se comporte exactement de la même manière que la surcharge const, de sorte que les arguments puissent être transférés 1: 1. Si votre logique diffère et que vous n'appelez pas la version const via <=>, vous pouvez envisager d'autres approches.

Cet article DDJ montre comment utiliser la spécialisation de modèles sans que vous ayez à le faire. const_cast. Pour une fonction aussi simple, ce n’est vraiment pas nécessaire.

boost :: any_cast (à un moment donné, ce n’est plus le cas) utilise un const_cast de la version const appelant la version non-const pour éviter les doublons. Vous ne pouvez pas imposer de sémantique const à la version non-const, vous devez donc être très prudent avec cela.

En fin de compte, la duplication de code est acceptable tant que les deux extraits sont directement superposés.

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

N'ayant pas trouvé ce que je cherchais, j'ai donc lancé deux de mes propres ...

Celle-ci est un peu verbeuse, mais présente l'avantage de gérer plusieurs méthodes surchargées du même nom (et du type de retour) en même temps: 

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 n'avez qu'une const méthode par nom, mais que vous avez encore beaucoup de méthodes à dupliquer, vous préférerez peut-être 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 s’efface dès que vous commencez à surcharger le nom (la liste des arguments de l’argument du pointeur de la fonction ne semble pas être résolue à ce stade, elle ne peut donc pas trouver de correspondance pour cet argument). Bien que vous puissiez vous en servir, vous pouvez également créer un modèle: 

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

Toutefois, les arguments de référence de la méthode <=> ne peuvent pas être comparés aux arguments apparemment by-value du modèle et il est rompu. Vous ne savez pas pourquoi. Voici pourquoi .

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top