this-Zeiger vom Besitzer der Cheshire Cat (Opaque Pointer, pImpl)



  • Die Diskussionsfrage habe ich dazu auch im anderen Thread aufgeworfen. Warum muss denn der Pointer smart sein? Und was macht denn das "smart" aus?



  • Private schrieb:

    Die Diskussionsfrage habe ich dazu auch im anderen Thread aufgeworfen. Warum muss denn der Pointer smart sein? Und was macht denn das "smart" aus?

    Dann schau mal genau in dem anderen Thread.

    Die Frage nach "callback an das pimpl-objekt übergeben" vs. "callback beim Funktionsaufruf übergeben" beschreibt Herb Sutter ebenfalls in dem Artikel, den ich Dir verlinkt habe.



  • Das war nicht mehr die Frage. Die Frage zum "Callback" ist bereits beantwortet, es bleibt nur noch die Frage, was die "Smartness" meiner beschriebenen Variante von der Variante mit unique_ptr unterscheidet.



  • Private schrieb:

    Das war nicht mehr die Frage. Die Frage zum "Callback" ist bereits beantwortet, es bleibt nur noch die Frage, was die "Smartness" meiner beschriebenen Variante von der Variante mit unique_ptr unterscheidet.

    std::unique_ptr erledigt automatisch das, was du manuell machst. In anderen Situationen sogar das, was du gar nicht manuell machen kannst (bzgl. Exceptionsicherheit). Es gibt keinerlei guten Argumente, delete jemals in Anwendungscode zu verwenden (Ausnahmesituationen gibt es, aber eine solche liegt hier nicht vor). Wieso stellst du dich auf stur mit so stichhaltigen Argumenten wie "seh keine Vorteile außer dass es einfacher ist"? Wieso programmierst du nicht gleich alles in Assembler, gibt ja schließlich keine Nachteile, außer "dass es in C++ einfacher ist".

    Mit der Einstellung wirst du nie ein besserer Entwickler. Willst du echt zu den paar Spezis gehören, die selbst nach 15 Jahren Berufserfahrung Code schreiben, der unsicher, nicht portabel, instabil, langsam und dreimal so kompliziert wie nötig ist?



  • ;qwerty schrieb:

    Private schrieb:

    Das war nicht mehr die Frage. Die Frage zum "Callback" ist bereits beantwortet, es bleibt nur noch die Frage, was die "Smartness" meiner beschriebenen Variante von der Variante mit unique_ptr unterscheidet.

    std::unique_ptr erledigt automatisch das, was du manuell machst. In anderen Situationen sogar das, was du gar nicht manuell machen kannst (bzgl. Exceptionsicherheit). Es gibt keinerlei guten Argumente, delete jemals in Anwendungscode zu verwenden (Ausnahmesituationen gibt es, aber eine solche liegt hier nicht vor). Wieso stellst du dich auf stur mit so stichhaltigen Argumenten wie "seh keine Vorteile außer dass es einfacher ist"? Wieso programmierst du nicht gleich alles in Assembler, gibt ja schließlich keine Nachteile, außer "dass es in C++ einfacher ist".

    Mit der Einstellung wirst du nie ein besserer Entwickler. Willst du echt zu den paar Spezis gehören, die selbst nach 15 Jahren Berufserfahrung Code schreiben, der unsicher, nicht portabel, instabil, langsam und dreimal so kompliziert wie nötig ist?

    Meine Erfahrung mit <memory> tendieren derzeitig gegen Null. Es ist für mich also ungleich schwerer, unique_ptr anstatt new/delete zu nutzen. Ich habe bereits versucht, die Implementierung mit unique_ptr zu ersetzen, allerdings crashte mein Programm immer (prinzipiell funktionierte die Implementierung, jedoch habe ich es nicht hinbekommen, einen einwandfrei funktionierenden Copy-Konstruktor und -Operator umzusetzen). Daher frage ich mich, warum ich mir diesen Aufwand antun soll, wenn das auch so funktioniert. Beruflich komme ich aus dem Bereich der Ausbildung und wenn ich da was gelernt habe, ist es, dass man bei Lernenden mit "Mach so, weil besser" nicht viel erreicht. Auch wenn vllt. das Ergebnis dann "richtiger" ist, so fehlt es immer noch am nachhaltigen Verständnis. Dass unique_ptr exceptionsicher ist, hatte ich sogar gelesen, allerdings nicht beachtet. Als Argument ist das schon nahezu ausreichend (schließlich würde meine Implementierung derzeitig crashen, wenn new/delete fehl schlügen).



  • Nein, da liegst du leider falsch.

    1. Deine Anwendungen müssen nicht zwangsläufig "crashen", wenn du new/delete falsch einsetzt. Ein doppeltes delete führt zu undefiniertem Verhalten, und das ist so ziemlich das Übelste, was deinem Programm passieren kann.

    2. Der Einarbeitungsaufwand ist nicht so groß, dass man ihn als Einwand gegen sauberes Programmieren aufführen kann. Die Zeit, die du jetzt in das Lernen eines vernünftigen Speichermanagements steckst sparst du später um ein Vielfaches wieder ein, wenn du nicht mit dem Debugger durch dein halbes Programm laufen musst, um Speicherlecks und undefiniertes Verhalten zu suchen.


  • Mod

    Ist dir denn wenigstens klar, wieso meine erste Reaktion zu deinem Code war, dass er falsch ist und ich damit nicht das fehlende delete meinte? Wenn du das nämlich nicht verstehst, dann mangelt es an grundlegendem Hintergrundwissen. Das wäre aber erst recht ein Grund, solche Dinge nicht selber zu machen, sondern fertige Lösungen zu bevorzugen.



  • SeppJ schrieb:

    Ist dir denn wenigstens klar, wieso meine erste Reaktion zu deinem Code war, dass er falsch ist und ich damit nicht das fehlende delete meinte? Wenn du das nämlich nicht verstehst, dann mangelt es an grundlegendem Hintergrundwissen. Das wäre aber erst recht ein Grund, solche Dinge nicht selber zu machen, sondern fertige Lösungen zu bevorzugen.

    Wenn sich das darauf bezogen hatte, dass ich keine Smart-Pointer verwende, dann ja. Ansonsten freue ich mich auch über eine Erklärung


  • Mod

    Was passiert denn, wenn ich so ein Objekt kopiere oder einem anderen zuweise, wenn man das so macht, wie du das programmiert hast?



  • So wie ich es oben gepostet habe? Prinzipiell wird der Standard-Copy-Konstruktor genutzt. Dieser wird dann den Pointer cc vom "alten" Objekt zum neuen kopieren. Ist also eine einfach Pointer-Kopie. Sollte das alte Objekt dann gelöscht werden, wird die Adresse im neuen Objekt ins nichts zeigen, wodurch undefiniertes Verhalten entsteht bzw. das Programm crasht, falls auf das cc vom neuen Objekt zugegriffen werden soll. Ähnlich bei der Zuweisung: weise ich dem neuen cc einen Wert zu bzw. ändere den, so wird auch der vom alten geändert.


  • Mod

    Und wie löst du dieses Problem?



  • Um beim Beispiel zu bleiben:

    class PUBLIC {
    public:
    	PUBLIC ();
    	PUBLIC (PUBLIC&);
    	PUBLIC& operator = (PUBLIC);
    
    	~PUBLIC ();
    
    	int get ();
    	void set (int);
    
    private:
    	struct CheshireCat;
    	CheshireCat *cc;
    };
    
    struct PUBLIC::CheshireCat {
    	int b;
    };
    
    int PUBLIC::get () {
    	return this->cc->b;
    }
    
    void PUBLIC::set (int value) {
    	this->cc->b = value;
    }
    
    PUBLIC::PUBLIC () : cc (new CheshireCat) {
    }
    
    PUBLIC::PUBLIC (PUBLIC& origin) : cc (new CheshireCat (*(origin.cc))) {
    }
    
    PUBLIC& PUBLIC::operator = (PUBLIC origin) {
    	delete this->cc;
    	this->cc = new CheshireCat (*(origin.cc));
    
    	return *this;
    }
    
    PUBLIC::~PUBLIC () {
    	delete this->cc;
    }
    


  • Hättest du std::unique_ptr verwendet, hätte das automatisch den Kopierkonstruktor und den assignment operator gelöscht, sowie einen geeigneten Move-Konstruktor und move assignment operator angelegt (die hier nach wie vor fehlen) - ohne auch nur eine einzige Zeile Code schreiben zu müssen.

    Schau dir außerdem nochmal an, wie die Signaturen des Kopierkonstruktors und des Zuweisungsoperators aussehen müssen.


  • Mod

    Und wieso hast du es nicht gleich so gemacht? Zu viel Arbeit oder einfach nicht gewusst? Beides wäre bei unique_ptr entfallen.

    Außerdem ist deine Zuweisung falsch ungeschickt.



  • SeppJ schrieb:

    Und wieso hast du es nicht gleich so gemacht? Zu viel Arbeit oder einfach nicht gewusst? Beides wäre bei unique_ptr entfallen.

    Außerdem ist deine Zuweisung falsch ungeschickt.

    Zu viel Arbeit, wenn man so will. Das Kopieren stand für mich nicht an erster Stelle und habe es daher erst später implementiert. Ich muss mir einige Zusammenhänge erst erarbeiten (in dem Fall Opaque Pointer bzw. pImpl), weshalb ich Schritt für Schritt eine Funktionalität nach der anderen umsetze.
    C++x11 ist für mich ziemliches Neuland, sodass ich mir Dinge wie member initializer lists und smart pointer eben erarbeiten muss. Ich werde auch unique_ptr nutzen, kenne jetzt aber auch den "harten" Weg (den Weg vor C++x11).
    Könntest du noch beschreiben, was an welcher Zuweisung ungeschickt ist?

    @;qwerty: Danke für den Hinweis. Der Copy-Konstruktor war ja soweit korrekt (gemäß http://cpp0x.centaur.ath.cx/class.copy.html), allerdings fehlte beim Copy-Operator noch was...



  • Private schrieb:

    Der Copy-Konstruktor war ja soweit korrekt (gemäß http://cpp0x.centaur.ath.cx/class.copy.html)

    Welchen praktischen Nutzen soll ein Kopierkonstruktor haben, der keine const Referenzen annehmen kann?

    Private schrieb:

    allerdings fehlte beim Copy-Operator noch was...

    Eher war da zu viel. Du hast die Interna von einer Kopie gleich noch einmal kopiert. Entweder operator=(const Foo&) und kopieren oder operator=(Foo) und swappen/moven. Aber keine Mischformen.


  • Mod

    Dein Zuweisungsoperator hat folgende Schwächen:
    1. Es wird zweimal new aufgerufen. Glaubst du nicht? Denk mal genau nach, was die Funktion macht.
    2. Die alte Ressource wird freigegeben, bevor du die neue besorgst. Es ist aber nicht sicher, ob das Besorgen der neuen Ressource klappt (new kann fehlschlagen!). Falls es fehlschlägt, hinterlässt du ein Objekt in einem halb-gültigen Zustand (oder kurz gesagt: Ungültig!). Das völlig ohne Not, denn es ginge, wie ich gleich erkläre, auch ohne diesen Nachteil und wäre sogar einfacher umzusetzen*.

    Wie geht es besser? "Copy&Swap" ist das Stichwort. Wir nutzen einfach deine erste, versteckte Ressourcenallokation (hast du sie schon gefunden?) und den automatischen Aufruf des Destruktors:

    PUBLIC& PUBLIC::operator = (PUBLIC origin) {
        std::swap(cc, origin.cc); 
        return *this;
    }
    

    Bumm! Zwei Zeilen Code, alles erledigt, in jeder Hinsicht besser als das was vorher da war.

    Aber: In diesem Fall machen wir uns das Leben immer noch viel zu schwer. Obiges ist die Lösung für den allgemeinen Fall, in dem man Ressourcen nicht wiederverwenden kann. Hier habe wir ja ein zuweisbares Objekt, das wir kopieren wollen und wir haben ein altes Objekt dieser Klasse. Daher noch einfacher, ganz ohne new, auch ohne verstecktes:

    PUBLIC& PUBLIC::operator = (const PUBLIC &origin) {
        *cc = *origin.cc; 
        return *this;
    }
    

    Und die Arbeit, falls es überhaupt welche zu tun gibt, übernimmt dann der Zuweisungsoperator der Klasse CheshireCat.

    PS: Der Copy-Konstruktor sollte wohl eher eine const-Referenz als Argument haben, findest du nicht? Es sind diese vielen kleinen Fehler die du in deiner eigenen Implementierung machst, die du dir komplett gespart hättest, wenn du gleich einen Smartpointer genommen hättest. Wie lange hat das Programmieren + Fehlerbehebung gedauert? Wie lange hätte es gedauert, sich unique_ptr anzusehen? Denk dran, dass du die Fehler nicht einmal selber entdecken und analysieren musstest, sie wurden dir direkt genannt. Wenn du nicht hier im Forum gefragt hättest, würdest du wahrscheinlich nächste Woche noch nach der Ursache für die komischen Segfaults suchen.

    *: Einfacher Umzusetzen ist das Thema des Tages. unique_ptr wäre besser gewesen und einfacher umzusetzen. Copy&Swap wäre besser gewesen und einfacher umzusetzen. Du machst dir hier die ganze Zeit große Mühe, um am Ende schlechtere Lösungen zu erhalten und denkst dabei noch, dass dein Weg einfacher wäre.



  • Private schrieb:

    Zu viel Arbeit, wenn man so will. Das Kopieren stand für mich nicht an erster Stelle und habe es daher erst später implementiert. Ich muss mir einige Zusammenhänge erst erarbeiten (in dem Fall Opaque Pointer bzw. pImpl), weshalb ich Schritt für Schritt eine Funktionalität nach der anderen umsetze.
    C++x11 ist für mich ziemliches Neuland, sodass ich mir Dinge wie member initializer lists und smart pointer eben erarbeiten muss. Ich werde auch unique_ptr nutzen, kenne jetzt aber auch den "harten" Weg (den Weg vor C++x11).
    [...]

    Ich kann Privates Vorgehen in gewisser Weise nachvollziehen, denn es geht ja um verschiedene unabhängige Themen:
    1. Compiler-firewall (mit Hilfe von Pimpl/Grinsekatze)
    2. Resourcenverwaltung mit Smart-Pointern / RAII
    3. Deep-Copy bzw. -Move
    wobei es ihm primär ja um 1. ging. Da will man zuerst man eine Sache vollständig verstehen, was man macht, und nicht zwei neue Themen gleichzeitig lernen.

    Es wäre natürlich geschickter gewesen, das Thema Smart-Pointer zuerst zu kennen und anwenden zu können, und mit diesem Wissen dann Pimpl zu implementieren. Konnte er tatsächlich nicht wissen.

    Nichts desto trotz finde ich Smart-Pointer/RAII ganz allgemein extrem wichtig und es wert, umfassend zu lernen und anzuwenden.

    Im übrigen gab es auch vor C++11 bereits die auto_ptr , mit denen man Pimpl ebenfalls umsetzen konnte.



  • Man brauch für ne Compilation-Firewall auch nicht unbedingt Smart-Pointer. (OK, dass es ohne geht ist sowieso klar. Ich meine: ich finde der Code wird nichtmal sehr arg kompliziert/hässlich/redundant wenn man keine Smart-Pointer verwendet.)

    Die von SeppJ gezeigte Variante

    PUBLIC& PUBLIC::operator = (const PUBLIC &origin) {
        *cc = *origin.cc; 
        return *this;
    }
    

    funktioniert auch ohne Smart Pointer.

    Man muss dazu nur die grossen 3/5 selbst implementieren.
    Was aber relativ trivial ist.
    Einzig die Exception-Safety im Ctor könnte lästig werden. Dafür kann man sich dann eine "Pointer Guard" Hilfsklasse schreiben:

    template <class T>
    struct PointerGuard
    {
        T* p;
    
        explicit PointerGuard(T* p = 0) : p(p) {}
        ~PointerGuard() { delete p; }
    
        PointerGuard(PointerGuard const&) = delete;
        PointerGuard& operator = (PointerGuard const&) = delete;
    };
    

    Für die Exception-Safety im Ctor ist das ausreichend.

    std::auto_ptr ist natürlich auch eine Option, aber das wäre dann ja schon wieder ein Smart-Pointer. Dann könnte man auch gleich std::unique_ptr oder boost::scoped_ptr verwenden.

    Wobei mir gerade einfällt... es gab mal (zumindest) einen Thread hier im Forum zum Thema "Smart Pointer der sich beim Kopieren/Zuweisen wie das Objekt selbst verhält".

    Der also im Assignment-Operator nicht den Zeiger ändert sondern *p = *other.p macht und im Copy-Ctor nicht den Zeiger kopiert sondern eben p(new T(*p)) macht.
    Für die meisten VerPIMPeLungen wäre das genau das richtige Verhalten. Kann sogar sein dass diese Art Smart-Pointer damals genau in diesem Zusammenhang diskutiert wurde.



  • hustbaer schrieb:

    std::auto_ptr ist natürlich auch eine Option, aber das wäre dann ja schon wieder ein Smart-Pointer. Dann könnte man auch gleich std::unique_ptr oder boost::scoped_ptr verwenden.

    Es war ja von den Zeiten vor C++11 die Rede...

    auto_ptr direkt im Vergleich mit unique_ptr wäre schlechter, weil er die const-ness nicht richtig macht.

    hustbaer schrieb:

    Wobei mir gerade einfällt... es gab mal (zumindest) einen Thread hier im Forum zum Thema "Smart Pointer der sich beim Kopieren/Zuweisen wie das Objekt selbst verhält".

    Der also im Assignment-Operator nicht den Zeiger ändert sondern *p = *other.p macht und im Copy-Ctor nicht den Zeiger kopiert sondern eben p(new T(*p)) macht.
    Für die meisten VerPIMPeLungen wäre das genau das richtige Verhalten. Kann sogar sein dass diese Art Smart-Pointer damals genau in diesem Zusammenhang diskutiert wurde.

    Meinst Du den hier:
    https://github.com/sqjk/pimpl_ptr/blob/master/include/pimpl_ptr.hh

    Der bringt ein paar nette Sachen mit (z.B. man braucht das new nicht selbst zu schreiben) und verwendet statt einem Pointer ein std::aligned_storage . Allerdings muss man in der Deklaration die Größe der eingebundenen Datenklasse wissen, was der eigentlichen Firewall-Idee etwas zuwiderläuft. Letzteres fände ich jetzt wieder doof.


Anmelden zum Antworten