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

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

  • 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.

    0
  • ?

    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...

    0
  • ?

    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.

    0
  • 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.

    0
  • M

    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.

    0

  • 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.

    0
  • M

    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.

    0

  • minastaros schrieb:

    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.

    Klingt für mich jetzt so als ob du irgendwas aus meinem Beitrag korrigieren wollen würdest. Ich komme aber ums verrecken nicht drauf was.

    minastaros schrieb:

    Meinst Du den hier:

    Nö, ich meine wirklich einen Thread hier im Forum wo es glaube ich darum ging dass jmd. einen solchen Smart-Pointer entwickelt wollte.
    Mag aber sein dass ich mich täusche und es um einen Wollmilchsau-Smartpointer ging, und das beschriebene Verhalten nur eines von vielen möglichen war die per Traits einstellbar sein sollten. Irgendwas in der Art.
    Auf jeden Fall nix mit std::aligned_storage .

    minastaros schrieb:

    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.

    Ja, etwas schon. Aber nicht so wild.
    Man kann ja - speziell in der Entwicklungsphase - mal grosszügig ein paar zig Bytes mehr hinschreiben als man aktuell braucht. Dann muss man auch nur sehr selten neu übersetzen. Und bei sizeof/alignof-neutralen Änderungen sowieso gar nicht.

    0
  • ?

    SeppJ schrieb:

    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!).

    Doch, ich glaube dir. Ich meine, dass origin.cc intern "kopiert" wird. Es wird also nochmal new aufgerufen um eine temporäre Kopie zu erstellen, so dass - falls mein new CheshireCat (*(origin.cc)) fehlschlägt - wenigstens origin.cc nicht ungültig wird.
    Dein zweiter Punkt ist natürlich auch vollkommen berechtigt. Und danke für die weiteren Ausführungen (Vereinfachungen).

    Auch euch (;qwerty, minastaros und hustbaer) vielen Dank für die Anmerkungen/Stellungnahmen.

    hustbaer schrieb:

    minastaros schrieb:

    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.

    Klingt für mich jetzt so als ob du irgendwas aus meinem Beitrag korrigieren wollen würdest. Ich komme aber ums verrecken nicht drauf was.

    Ich denke, sein Einwand bezog sich darauf, dass ich es habe klingen lassen, als wenn es Smart Pointer erst seit C++11 gibt (daher Verweis auf auto_ptr). Und da ich von Zeiten vor C++11 gesprochen habe, wäre zumindest der Verweis auf unique_ptr nicht treffend.

    0

  • @Private

    (...)

    Ich denke, sein Einwand bezog sich darauf, dass ich es habe klingen lassen, als wenn es Smart Pointer erst seit C++11 gibt (daher Verweis auf auto_ptr). Und da ich von Zeiten vor C++11 gesprochen habe, wäre zumindest der Verweis auf unique_ptr nicht treffend.

    Naja deswegen hab ich ja auch "oder boost::scoped_ptr " geschrieben.
    Aber egal 🙂

    0
  • M

    hustbaer schrieb:

    @Private

    (...)

    Ich denke, sein Einwand bezog sich darauf, dass ich es habe klingen lassen, als wenn es Smart Pointer erst seit C++11 gibt (daher Verweis auf auto_ptr). Und da ich von Zeiten vor C++11 gesprochen habe, wäre zumindest der Verweis auf unique_ptr nicht treffend.

    Naja deswegen hab ich ja auch "oder boost::scoped_ptr " geschrieben.
    Aber egal 🙂

    Genau so war's. Na dann sind wir ja jetzt alle wieder auf dem gleichen Stand.
    Einen schönen Tag noch und vielen Dank für die Fische! 😉

    0
Anmelden zum Antworten