Wie entferne ich Codeduplizierungen zwischen ähnlichen konstanten und nicht konstanten Mitgliedsfunktionen?

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

Frage

Nehmen wir an, ich habe Folgendes 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 Mitgliedsfunktionen X::Z() Und X::Z() const haben identischen Code in den geschweiften Klammern.Dies ist doppelter Code und kann bei langen Funktionen mit komplexer Logik zu Wartungsproblemen führen.

Gibt es eine Möglichkeit, diese Codeduplizierung zu vermeiden?

War es hilfreich?

Lösung 2

Ja, ist es möglich, den Code Doppelarbeit zu vermeiden. Sie müssen die konstante Elementfunktion verwenden, um die Logik zu haben, und haben das nicht konstante Elementfunktion rufen Sie die konstante Elementfunktion und wieder werfen den Rückgabewert auf eine nicht konstante Referenz (oder Zeiger, wenn die Funktionen einen Zeiger zurückgibt):

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 Sie das tun nicht setzen die Logik in der nicht-konstante Funktion und haben die const-Funktion aufrufen, die nicht konstante Funktion - es kann zu undefinierten Verhalten führen. Der Grund dafür ist, dass eine konstante Klasseninstanz als nicht-ständige Instanz gegossen wird. Die nicht-const Member-Funktion die Klasse versehentlich ändern kann, die die C ++ Standardzustände in undefinierten Verhalten führen.

Andere Tipps

Für eine detaillierte Erklärung finden Sie in der Rubrik „Vermeiden Vervielfältigung in const und Nicht const Member-Funktion“ auf S.. 23, in Punkt 3 "Use const, wann immer möglich," in Effective C ++ , 3d ed 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 Würfe und Funktionsaufruf können hässlich sein, aber es ist richtig. Meyers hat eine ausführliche Erklärung, warum.

Ich denke, Scott Meyers' Lösung C ++ 11 unter Verwendung einer tempate Helferfunktion verbessert werden kann. Dies macht die Absicht viel offensichtlicher und kann für viele andere Getter wiederverwendet 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 die folgende Art und 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 Elementfunktion zu rufen. Nach dass eine beliebige Menge von zusätzlichen Argumente übergeben werden, so dass sie an die Funktion weitergeleitet werden kann. Dies muss C ++ 11 wegen der variadische Vorlagen.

Ein wenig ausführlicher als Meyers, aber ich könnte dies 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 es eine nicht-const Z & für eine const-Instanz zurückgibt, weshalb es privat ist. Private Verfahren können Invarianten der externen Schnittstelle brechen (in diesem Fall die gewünschte invariant ist „ein const Objekt nicht über Referenzen durch sie auf Objekte hat-a erhalten modifiziert werden“).

Beachten Sie, dass die Kommentare Teil des Musters sind - _getZ der Schnittstelle gibt an, dass es nie gültig ist, zu nennen es (abgesehen von dem Zugriffs-, natürlich): Es gibt keinen denkbaren Nutzen so ohnehin zu tun, denn es ist ein mehr Charakter zu geben und nicht in kleinerem oder schnelleren Code führen. Aufruf der Methode ist äquivalent mit einem const_cast einer der Accessoren zu nennen, und Sie würde nicht wollen, dass entweder zu tun. Wenn Sie besorgt sind über die Herstellung Fehler offensichtlich (und das ist ein gutes Ziel), dann nennt es const_cast_getZ statt _getZ.

Übrigens, ich schätze Meyers-Lösung. Ich habe keine philosophischen Einwände dagegen. Persönlich, obwohl ich lieber ein klein wenig kontrollierte Wiederholung und eine private Methode, die nur in bestimmten streng kontrollierten Umständen aufgerufen werden muss, über ein Verfahren, das wie Leitungsrauschen aussieht. Wählen Sie Ihr Gift und bleiben Sie dabei.

[Edit: Kevin hat zu Recht darauf hingewiesen, dass _getZ könnte ein weiteres Verfahren nennen will (sagen generateZ), die const-spezialisiert auf die gleiche Art und Weise ist Getz ist. In diesem Fall würde sehen _getZ ein const Z & und hat es const_cast vor der Rückkehr. Das ist immer noch sicher, da die vorformulierten Accessor alles polizeilich überwacht, aber es ist nicht hervorragend klar, dass es sicher ist. Außerdem, wenn Sie das tun und dann später generateZ ändern, um immer const zurück, dann ändern Sie müssen auch Getz immer const zurück, aber der Compiler wird Ihnen nicht sagen, dass Sie das tun.

Die letztgenannte Punkt, um den Compiler ist auch wahr, die empfohlenen Muster des Meyers, aber der erste Punkt, um eine nicht-offensichtliche const_cast nicht. Also unter dem Strich denke ich, dass, wenn _getZ stellt sich heraus, ein const_cast für den Rückgabewert zu müssen, dann ist dieses Muster viel von seinem Wert über Meyers verliert. Da es auch Nachteile auf Meyers verglichen leidet, glaube ich an seinen in dieser Situation wechseln würde. leicht Refactoring von einem zum anderen ist -. es keinen anderen gültigen Code in der Klasse beeinflussen, da nur ungültigen Code und die Textvorschlag nennt _getZ]

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 folgende Vorteile:

  • Es ist offensichtlich, was los ist
  • Hat einen minimalen Code-Overhead – er passt in eine einzige Zeile
  • Es ist schwer, etwas falsch zu machen (man kann es nur wegwerfen). volatile zufällig, aber volatile ist ein seltenes Qualifikationsmerkmal)

Wenn Sie den vollständigen Abzugsweg nutzen möchten, können Sie dies mithilfe einer 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 können Sie nicht einmal etwas vermasseln volatile, und die Verwendung sieht so aus

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

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

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

};

Allerdings hat es die Hässlichkeit ein statisches Element zu erfordern und die Notwendigkeit, die instance Variable in der Verwendung es.

Ich hielt es nicht für alle möglichen (negativen) Auswirkungen dieser Lösung. Bitte lassen Sie mich wissen, wenn überhaupt.

Sie können auch lösen diese mit Vorlagen. Diese Lösung ist leicht hässlich (aber die Hässlichkeit ist in der CPP-Datei versteckt), aber es tut Überprüfung Compiler bietet von Konstantheit 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 ich sehen kann, ist, dass, weil alle die komplexe Implementierung des Verfahrens in einer globalen Funktion ist, müssen Sie entweder hält die Mitglieder der X wie getVector mit öffentlichen Methoden get () oben (von denen es immer braucht sein, um eine konstante und nicht konstante Version) oder Sie können diese Funktion ein Freund machen. Aber ich weiß nicht wie Freunde.

[Edit:. Entfernt nicht benötigte umfasst von cstdio während des Tests hinzugefügt]

Wie wäre es, die Logik in eine private Methode zu bewegen, und nur tun, die „den Verweis bekommen und zurück“ Sachen in den Getter? Eigentlich wäre ich ziemlich verwirrt über die statischen und konst wirft in einer einfachen Getter-Funktion, und ich würde das hässlich betrachtet Ausnahme äußerst seltene Fälle!

betrügt sie den Präprozessor benutzen?

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 Vorlagen oder Abgüsse, aber es Ihre Absicht machen ( „diese beiden Funktionen identisch sind“) ziemlich eindeutig.

Typischerweise werden die Member-Funktionen, für die Sie const und nicht-const-Versionen sind Getter und Setter müssen. Die meiste Zeit sind sie Einzeiler so Code-Duplizierung ist kein Problem.

ich dies für einen Freund tat, die zu Recht die Verwendung von const_cast gerechtfertigt ... nicht darüber zu wissen, ich wahrscheinlich so etwas wie diese (nicht wirklich elegant) getan hätte:

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

Ich würde eine private Helfer statische Funktion Vorlage vorschlagen, wie folgt aus:

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

Für diejenigen (wie mich), die

  • Verwendung c ++ 17
  • will die geringste Menge an vorformulierten / Wiederholung und
  • hinzufügen
  • nicht dagegen, mit Makros (beim Warten auf Meta-Klassen ...)

hier ist eine andere nehmen:

#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 fügt Tisch ist, dass man mit nur einem zusätzlichen Codezeile weg, die einfach Name der Funktion (aber kein Argument oder Typ Vervielfältigung zurück):

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

Hinweis: gcc versagt diese vor 8.1, klirren-5 und aufwärts sowie MSVC-19 sind glücklich (nach der Compiler-Explorer ).

Es ist mir überraschend, dass es so viele verschiedene Antworten, aber fast alle verlassen sich auf schwere Vorlage Magie. Vorlagen sind leistungsstark, aber manchmal Makros schlagen sie in Prägnanz. Maximale Vielseitigkeit wird oft erreicht, indem beide kombiniert werden.

Ich schrieb ein Makro FROM_CONST_OVERLOAD(), die in der nicht-konstante Funktion gesetzt werden kann die konstante Funktion aufzurufen.

Beispiel Nutzung:

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)

Erklärung:

Wie in vielen Antworten geschrieben, das typische Muster Code-Duplizierung in einer nicht konstanten Elementfunktion zu vermeiden, ist dies:

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

Viele dieser vorformulierten kann mit Typinferenz vermieden werden. Erstens kann const_cast in WithoutConst() verkapselt werden, die den Typ des Arguments folgert und entfernt das const-Qualifizierer. Zweitens wird ein ähnlicher Ansatz kann in WithConst() verwendet werden, um den this Zeiger auf const zu qualifizieren, die den Aufruf der Methode const-überlastet werden können.

Der Rest ist ein einfaches Makro, der das Gespräch mit dem korrekt qualifizierten this-> Präfixen und entfernt konst aus dem Ergebnis. Da der Ausdruck im Makro verwendete fast immer ein einfacher Funktionsaufruf ist mit 1: 1 weitergeleitet Argumenten, Nachteile von Makros wie Mehrfachauswertung treten nicht in den Ellipsen und __VA_ARGS__ könnten auch verwendet werden, aber nicht, weil Komma benötigt werden (. als Argument Separatoren in Klammern auftreten).

Dieser Ansatz hat mehrere Vorteile:

  • Minimal und natürliche Syntax - nur den Anruf in FROM_CONST_OVERLOAD( ) wickeln
  • Keine zusätzliche Member-Funktion erforderlich
  • Kompatibel mit C ++ 98
  • Einfache Implementierung, keine Metaprogrammierung und Null-Abhängigkeiten
  • Extensible: andere const Beziehungen können (wie const_iterator, std::shared_ptr<const T>, etc.) hinzugefügt werden. Dazu einfach WithoutConst() für die entsprechenden Typen überlasten.

Einschränkungen: Diese Lösung für Szenarien optimiert wird, wo die nicht konstante Überlastung ist genau wie die const Überlastung das gleiche tun, so dass Argumente 1 weitergeleitet werden können: 1. Wenn Ihre Logik unterscheidet und Sie sind nicht die const-Version über this->Method(args) aufrufen, können Sie andere Ansätze berücksichtigen.

Diese DDJ Artikel zeigt eine Art und Weise Template-Spezialisierung verwenden, erfordert Sie nicht zu verwenden, const_cast. Für eine solche einfache Funktion ist es wirklich nicht wenn nötig.

boost :: any_cast (an einem Punkt, ist es nicht mehr) einen const_cast aus der const-Version verwendet die nicht-const Version Aufruf Doppelarbeit zu vermeiden. Sie können nicht const Semantik auf der nicht-const Version aufzuzwingen aber so muss man sein sehr vorsichtig damit.

Am Ende einige Code-Duplizierung ist in Ordnung, solange die beiden Schnipsel direkt auf der jeweils anderen sind.

zur Lösung jwfearn hinzuzufügen und kevin zur Verfügung gestellt, ist 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;
};

Sie haben nicht gefunden, was ich suchte, so rollte ich ein paar meiner eigenen ...

Dies ist einer ein wenig wortreich, hat aber den Vorteil, viele überladenen Methoden mit dem gleichen Namen Handhabung (und Rückgabetyp) auf einmal:

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, aber immer noch viele Methoden zu duplizieren, dann könnten Sie diese bevorzugen:

  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 diese ab, sobald Sie den Namen Überlastung starten (das Funktionsargument Liste des Zeigerargument scheint an diesem Punkt ungelöst zu sein, so kann es keine Übereinstimmung für das Funktionsargument finden). Auch wenn Sie Ihren Weg aus dieser Vorlage kann, auch:

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

Aber Referenz Argumente an die const Methode nicht gegen die offenbar von Wert Argumente auf die Vorlage übereinstimmen und es bricht. nicht sicher, warum. Hier ist der Grund .

Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top