wake-up-neo.net

Wie entferne ich Code-Duplizierung zwischen ähnlichen const- und non-const-Memberfunktionen?

Angenommen, ich habe den folgenden class X, wo ich den Zugriff auf ein internes Mitglied zurückgeben möchte:

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

Die beiden Elementfunktionen X::Z() und X::Z() const haben einen identischen Code in den Klammern. Dies ist doppelter Code und kann zu Wartungsproblemen bei langen Funktionen mit komplexer Logik führen. 

Gibt es eine Möglichkeit, diese Code-Duplizierung zu vermeiden?

203
Kevin

Ja, es ist möglich, die Code-Duplizierung zu vermeiden. Sie müssen die const-Elementfunktion verwenden, damit die Logik und die Nicht-const-Elementfunktion die const-Elementfunktion aufrufen und den Rückgabewert in eine Nicht-const-Referenz umwandeln (oder einen Zeiger, wenn die Funktionen einen Zeiger zurückgeben):

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

HINWEIS: Es ist wichtig, dass SieNICHTdie Logik in die Non-const-Funktion setzen und die const-Funktion die non-const-Funktion aufrufen lassen. Dies kann zu undefiniertem Verhalten führen. Der Grund ist, dass eine konstante Klasseninstanz als nicht konstante Instanz umgewandelt wird. Die Nicht-Konstanten-Member-Funktion kann die Klasse versehentlich ändern, wodurch die C++ - Standardzustände zu undefiniertem Verhalten führen.

54
Kevin

Eine ausführliche Erklärung finden Sie in der Überschrift "Vermeidung von Duplikationen in const- und Nicht-const-Member-Funktion" auf Seite. 23, in Punkt 3 "Verwenden Sie const wann immer möglich" in Effective C++ , 3d von Scott Meyers, ISBN-13: 9780321334879.

alt text

Hier ist Meyers 'Lösung (vereinfacht):

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

Die beiden Besetzungen und der Funktionsaufruf sind möglicherweise hässlich, aber es ist richtig. Meyers hat eine gründliche Erklärung dafür.

160
jwfearn

Ich denke, die Lösung von Scott Meyers kann in C++ 11 mithilfe einer temporären Hilfsfunktion verbessert werden. Dies macht die Absicht viel offensichtlicher und kann für viele andere Getter verwendet werden.

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

Diese Hilfsfunktion kann auf folgende Weise verwendet werden.

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

Das erste Argument ist immer der this-Zeiger. Der zweite ist der Zeiger auf die aufzurufende Memberfunktion. Danach können beliebig viele zusätzliche Argumente übergeben werden, so dass sie an die Funktion ..__ weitergeleitet werden können.

30
Pait

Ein bisschen ausführlicher als Meyers, aber ich könnte Folgendes tun:

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

Die private Methode hat die unerwünschte Eigenschaft, dass sie für eine const-Instanz eine nicht konstante Z & amp; zurückgibt, weshalb sie privat ist. Private Methoden können Invarianten des externen Interfaces durchbrechen (in diesem Fall ist die gewünschte Invariante "ein const-Objekt kann nicht über Referenzen, die über es auf Objekte abgerufen werden, die es hat, geändert werden" geändert werden).

Beachten Sie, dass die Kommentare Teil des Musters sind - das Interface von _getZ gibt an, dass es niemals gültig ist, es aufzurufen (abgesehen von den Accessoren natürlich): Es ist ohnehin kein Vorteil, da dies ein Zeichen ist, das eingegeben werden muss und nicht führt zu kleinerem oder schnellerem Code. Das Aufrufen der Methode entspricht dem Aufrufen eines der Zugriffsmethoden mit const_cast. Dies möchten Sie auch nicht. Wenn Sie sich Sorgen machen, Fehler offensichtlich zu machen (und das ist ein faires Ziel), dann nennen Sie es const_cast_getZ anstelle von _getZ.

Ich schätze übrigens die Lösung von Meyers. Ich habe keinen philosophischen Einwand dagegen. Ich persönlich bevorzuge jedoch eine kleine kontrollierte Wiederholung und eine private Methode, die nur unter bestimmten, streng kontrollierten Umständen aufgerufen werden muss, gegenüber einer Methode, die wie Leitungsrauschen aussieht. Wähle dein Gift und bleib dabei.

[Bearbeiten: Kevin hat zu Recht darauf hingewiesen, dass _getZ möglicherweise eine weitere Methode aufrufen möchte (etwa generierenZ), die auf die gleiche Weise wie getZ spezialisiert ist. In diesem Fall würde _getZ ein const Z & sehen und muss es vor dem Zurückkehren const_cast darstellen. Das ist immer noch sicher, da der Boilerplate-Accessor alles kontrolliert, aber es ist nicht offensichtlich, dass es sicher ist. Darüber hinaus müssen Sie getZ so ändern, dass Sie const immer zurückgeben, wenn Sie das dann tun und später generieren, um "const" zurückzugeben. Der Compiler sagt jedoch nicht, dass Sie dies tun.

Dieser letzte Punkt bezüglich des Compilers trifft auch auf Meyers 'empfohlenes Muster zu, der erste Punkt bei einem nicht naheliegenden const_cast nicht. Alles in allem denke ich, wenn _getZ einen const_cast als Rückgabewert benötigt, dann verliert dieses Muster viel von seinem Wert gegenüber dem von Meyers. Da es im Vergleich zu Meyers auch Nachteile hat, denke ich, dass ich in dieser Situation zu ihm wechseln würde. Das Umgestalten von einem zum anderen ist einfach - es wirkt sich nicht auf anderen gültigen Code in der Klasse aus, da nur ungültiger Code und die Boilerplate _getZ aufrufen.]

19
Steve Jessop

Schöne Frage und nette Antworten. Ich habe eine andere Lösung, die keine Abgüsse verwendet:

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

};

Es ist jedoch hässlich, ein statisches Element zu benötigen und die Variable instance darin zu verwenden.

Ich habe nicht alle möglichen (negativen) Auswirkungen dieser Lösung berücksichtigt. Bitte lass es mich wissen.

15
gd1

C++ 17 hat die beste Antwort auf diese Frage aktualisiert:

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

Das hat die Vorteile, dass es:

  • Ist offensichtlich was los ist
  • Hat minimalen Code-Aufwand - passt in eine einzige Zeile
  • Ist schwer zu verwechseln (kann nur volatile durch Zufall wegwerfen, aber volatile ist ein seltener Qualifikator)

Wenn Sie den vollständigen Abzugsweg durchführen möchten, können Sie dies durch eine Hilfsfunktion erreichen

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;

Jetzt kannst du nicht mal volatile versauen, und die Verwendung sieht so aus

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

Sie können dies auch mit Vorlagen lösen. Diese Lösung ist etwas hässlich (die Hässlichkeit ist jedoch in der .cpp-Datei versteckt), sie bietet jedoch eine Compiler-Überprüfung der Konstanz und keine Code-Duplizierung.

.h Datei:

#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-Datei:

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

Der Hauptnachteil, den ich sehen kann, ist, dass Sie, da sich die komplexe Implementierung der Methode in einer globalen Funktion befindet, entweder die Mitglieder von X mithilfe öffentlicher Methoden wie GetVector () benötigen (von denen immer ein const und non-const version) oder Sie können diese Funktion zu einem Freund machen. Aber ich mag keine Freunde.

[Bearbeiten: Nicht mehr benötigtes Include von Cstdio wurde während des Tests hinzugefügt.]

6
Andy Balaam

Wie wäre es, wenn Sie die Logik in eine private Methode umsetzen und nur das Zeug zum Abrufen und Zurückgeben in den Gettern ausführen? Eigentlich wäre ich ziemlich verwirrt über die statischen und const-Umwandlungen in einer einfachen Getter-Funktion, und ich würde das als hässlich betrachten, abgesehen von extrem seltenen Umständen!

3
MP24

Für diejenigen (wie ich), die

  • benutze c ++ 17
  • wollen die geringste Menge an Boilerplate hinzufügen / Wiederholung und
  • macht es nichts aus, makros zu verwenden (während auf Metaklassen gewartet wird ...),

hier ist eine weitere Einstellung:

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

Es ist im Grunde eine Mischung aus den Antworten von @Pait, @DavidStone und @ sh1. Was es zu Tabelle hinzufügt, ist, dass Sie mit nur einer zusätzlichen Codezeile davonkommen, die einfach die Funktion benennt (aber kein Argument oder Rückgabetypduplikation):

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

Hinweis: gcc kann dies vor 8.1 nicht kompilieren, clang-5 und höher sowie MSVC-19 sind zufrieden (laut dem Compiler-Explorer ).

1
axxel

Ich tat dies für einen Freund, der die Verwendung von const_cast... zu Recht rechtfertigte, ohne es zu wissen, hätte ich wahrscheinlich etwas (nicht wirklich elegant) gemacht:

#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
matovitch

Ist es ein Betrug, den Präprozessor zu verwenden?

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

};

Es ist nicht so schick wie Templates oder Casts, aber es macht Ihre Absicht ("diese beiden Funktionen sollen identisch sein") ziemlich eindeutig.

1
user1476176

Es ist für mich überraschend, dass es so viele verschiedene Antworten gibt, aber fast alle von starker Template-Magie abhängig sind. Vorlagen sind mächtig, aber manchmal werden sie von Makros prägnant geschlagen. Maximale Vielseitigkeit wird oft durch die Kombination beider erreicht.

Ich habe ein Makro FROM_CONST_OVERLOAD() geschrieben, das in die Nicht-Konstanten-Funktion eingefügt werden kann, um die Konstanten-Funktion aufzurufen.

Beispielverwendung:

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

Einfache und wiederverwendbare Implementierung:

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)

Erläuterung:

Wie in vielen Antworten angegeben, lautet das typische Muster zur Vermeidung von Codeduplizierungen in einer Nicht-Konstanten-Member-Funktion wie folgt:

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

Ein Großteil dieser Boilerplate kann durch Typinferenz vermieden werden. Erstens kann const_cast in WithoutConst() eingekapselt werden, wodurch der Typ seines Arguments abgeleitet und das const-Qualifikationsmerkmal entfernt wird. Zweitens kann ein ähnlicher Ansatz in WithConst() verwendet werden, um den Zeiger this zu const-qualifizieren, wodurch der Aufruf der const-überladenen Methode ermöglicht wird.

Der Rest ist ein einfaches Makro, das dem Aufruf den korrekt qualifizierten this-> voranstellt und const aus dem Ergebnis entfernt. Da es sich bei dem im Makro verwendeten Ausdruck fast immer um einen einfachen Funktionsaufruf mit 1: 1-weitergeleiteten Argumenten handelt, treten Nachteile von Makros wie die Mehrfachauswertung nicht in Erscheinung. Die Auslassungspunkte und __VA_ARGS__ könnten ebenfalls verwendet werden, sollten es aber nicht sein erforderlich, da Kommas (als Argumenttrennzeichen) in Klammern stehen.

Dieser Ansatz hat mehrere Vorteile:

  • Minimale und natürliche Syntax - schließen Sie den Aufruf einfach in FROM_CONST_OVERLOAD( ) ein.
  • Keine zusätzliche Mitgliedsfunktion erforderlich
  • Kompatibel mit C++ 98
  • Einfache Implementierung, keine Template-Metaprogrammierung und keine Abhängigkeiten
  • Erweiterbar: Andere const-Relationen können hinzugefügt werden (wie const_iterator, std::shared_ptr<const T> usw.). Überladen Sie dazu einfach WithoutConst() für die entsprechenden Typen.

Einschränkungen: Diese Lösung ist für Szenarien optimiert, in denen die Nicht-Konstanten-Überladung genau das Gleiche tut wie die Konstanten-Überladung, sodass Argumente 1: 1 weitergeleitet werden können. Wenn sich Ihre Logik unterscheidet und Sie die const-Version nicht über this->Method(args) aufrufen, können Sie andere Ansätze in Betracht ziehen.

1
TheOperator

In der Regel sind die Member-Funktionen, für die Sie die Versionen const und non-const benötigen, Getter und Setter. Meistens handelt es sich um Einzeiler, sodass Code-Duplizierung kein Problem darstellt.

0
Dima

Um der Lösung jwfearn und kevin hinzuzufügen, folgt hier die entsprechende Lösung, wenn die Funktion shared_ptr zurückgibt:

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
Christer Swahn

Ich würde eine statische Funktionsvorlage für private Helfer vorschlagen:

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
dats

Ich habe nicht gefunden, wonach ich gesucht habe, also habe ich ein paar von mir selbst gerollt ...

Dies ist ein wenig wortreich, hat aber den Vorteil, viele überladene Methoden mit demselben Namen (und Rückgabetyp) auf einmal zu behandeln:

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

Wenn Sie nur eine const-Methode pro Namen haben, aber noch viele Methoden duplizieren, können Sie dies vorziehen:

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

Leider bricht dies zusammen, sobald Sie den Namen überladen (die Argumentliste des Funktionszeigerarguments scheint zu diesem Zeitpunkt nicht gelöst zu sein, sodass keine Übereinstimmung für das Funktionsargument gefunden werden kann). Obwohl Sie sich auch einen Ausweg daraus machen können:

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

Referenzargumente für die const-Methode passen jedoch nicht mit den anscheinend Argumenten mit dem nach Wert angegebenen Wert der Vorlage zusammen, und sie bricht. Nicht sicher warum.Hier ist der Grund .

0
sh1