Smart Pointer statt 'new' und 'delete'?

knivil, bring doch mal ein Argument, warum man RAII nicht verwenden sollte, obwohl es nicht wegzudiskutierende Vorteile bringt. Und zwar konkreter als nur "es ist nicht nötig" bzw. "es geht auch ohne".

Ich rede jetzt nicht von new/delete zur Implementierung irgendwelcher Datenstrukturen, und der Artikel auch nicht. Es geht um new/delete in normalem Anwendercode. Was gewinnt man, wenn man automatisierbare Arbeit von Hand erledigt?

TyRoXx

Skym0sh0 schrieb:

TyRoXx schrieb:

Shade Of Mine schrieb:

Der Grund warum man einen unique_ptr in einen Container packt, ist Faulheit. Wenn wir ptr container in der Standard Library hätten, dann hätten wir diese Diskussion hier nicht.

Rate mal warum PtrContainer nicht in std sind und niemals sein werden.

Das hätte ich gerne mal beantwortet.

Man hat PtrContainer nur erfunden, weil RValue-Referenzen fehlten. Die gibt es inzwischen, also sind die Dinger obsolet.

Skym0sh0 schrieb:

Und auch, wieso es gut oder schlecht ist eine Standardbibliothek groß (oder klein) zu machen.

Die verschiedenen Hersteller tun sich schon schwer genug damit die überschaubare Bibliothek von C++ korrekt zu implementieren. Lieber Qualität als Quantität. Was fehlt denn unbedingt?
Was passiert, wenn voreilig standardisiert wird, sieht man an iostream s.

Skym0sh0 schrieb:

Im Vergleich zu Java ist die Sprachbibliothek von C++ ja extremst mickrig.

Kann man in Java inzwischen in weniger als 10 Zeilen eine UTF-8-Datei in einen String einlesen?

TyRoXx schrieb:

Man hat PtrContainer nur erfunden, weil RValue-Referenzen fehlten. Die gibt es inzwischen, also sind die Dinger obsolet.

Nur schade, dass vector<unique_ptr<T> > in der Benutzung eine ganz andere Syntax als ptr_vector<T> hat und man für generischen Code lieber bei der Syntax von vector<T> bleibt, also fehlen weiterhin Sprachmittel und der ptr_vector ist notwendig.

Nexus

aherdfxcvyadfgadf schrieb:

Nur schade, dass vector<unique_ptr<T> > in der Benutzung eine ganz andere Syntax als ptr_vector<T> hat und man für generischen Code lieber bei der Syntax von vector<T> bleibt, also fehlen weiterhin Sprachmittel und der ptr_vector ist notwendig.

So generisch ist der Code ohnehin nicht, wenn PtrContainer im Spiel sind. Wie im früheren Post angetönt, ist die Schnittstelle an diversen Stellen unterschiedlich, und nicht selten sind diese Unterschiede auf C++98-Probleme zurückzuführen, die mittlerweile nicht mehr existieren (Exceptionsicherheit ohne Move-Semantik, die ganzen auto_ptr -Geschichten, ...). Daher wären langfristig C++11-Adapter sinnvoller als die veralteten PtrContainer.

Schön wäre sowieso, wenn sich für C++11 eine Bibliothek wie Boost bilden würde, aber mit weniger Rückwärtsgewandtheit, weniger TMP-Overengineering und etwas mehr Pragmatismus. Momentan läufts dauernd auf Selbst-Basteln heraus, sobald man irgendwas Modernes braucht...

hustbaer

@knivil
Das einzige (für mich) interessante Beispiel das du bringst sind die Quadtrees.

Denn beim SSE Container schreibst du selbst eine RAII Klasse (den SSE Container)... ich hoffe ich muss jetzt nicht weiter darauf eingehen warum das Beispiel für mich nicht gilt.

Und die Sache mit den Bibliotheken... wenn man mit Programmteilen interfacen muss die nicht mehr C++ sind, dann ist auch irgendwie klar dass man RAII nur begrenzt verwenden kann.
OK, es gilt natürlich trotzdem als Beispiel wo RAII nicht möglich ist, insofern ist das OK.

Aber um auf den Quadtree zurückzukommen: was soll "keine smart pointer, weil sie nicht noetig sind" heissen?
Nötig sind sie nie, man kann immer selbst delete in den Destruktor schreiben. Nur finde ich es schlecht es zu tun, weil es schlechter lesbar und schlechter wartbar ist.
Also warum ist da der Quadtree eine Ausnahme?

knivil

Nun, ich glaube wir reden von unterschiedlichen Sachen. Ich kritisiere das Dogma, keine rohen Zeiger, direktes new / delete zu verwenden. RAII ist fuer mich genauso ein Muss. Aber um deine Frage zu beantworten:

Der Einfachheit halber mit Binaerbaeumen klassisch:

struct node {
    node* left;
    node* right;
}
struct bin_tree {
    node* root;
    // some operations for constructing, destructing and inserting
}

Dem Dogma folgend koennen alle rohen Zeiger durch unique_ptr ersetzt werden und man spart sich die eigenen Aufrufe von delete :

struct node {
    unique_ptr<node> left;
    unique_ptr<node> right;
}
struct bin_tree {
    unique_ptr<node> root;
}

Soweit so gut, jetzt erhoehen wir die Anforderungen wie sie beispielsweise bei einem Baum/Graph mit raeumlicher Knotenzuordnung (spatial partition) gewuenscht ist:
1.) benachbarte Knoten sollen nah im Speicher liegen (Ausnutzen des Prozessorcache)
2.) es ist kein Baum mehr sondern ein planarer Graph

Fuer 1.) bedeutet es, wie kann ich einen Teilbaum innerhalb eines vorgefertigten Speicherblocks konstruieren und organisieren. Fuer 2.) bedeuted es, wie ich die Knoten organisiere, damit ich z.B. zyklische Referenzen beseitige. In beiden Faellen wird mir unique_ptr, shared_ptr oder weak_ptr keine grosse Hilfe sein.

Fuer 2.) ist eine einfache Loesung, Speicher vo Knoten auf Bau-/Graphebene zu verwalten anstatt auf Knotenebene:

struct bin_tree {
    std::vector<node*> nodes;

    ~bin_tree() {
         for(auto e: nodes) delete nodes;
    }
}

Auf Ebene der Knoten ist es manuelles Speichermanagment, auf Ebene des Benutzers, der nur bin_tree zu Gesicht bekommt, ist es RAII.

Mit Problem 1.) musst ich noch nicht loesen, doch so schoen Graphen auch sind, es ist hart, das letzte bischen Leistung herauszuholen.

knivil schrieb:

Fuer 2.) ist eine einfache Loesung, Speicher vo Knoten auf Bau-/Graphebene zu verwalten anstatt auf Knotenebene:
struct bin_tree {
    std::vector<node*> nodes;

    ~bin_tree() {
         for(auto e: nodes) delete nodes;
    }
}
Auf Ebene der Knoten ist es manuelles Speichermanagment, auf Ebene des Benutzers, der nur bin_tree zu Gesicht bekommt, ist es RAII.

Warum nicht

struct bin_tree {
    std::vector<std::unique_ptr<node>> nodes;
}

? Oder meinethalben auch einen boost::ptr_vector, das ist dann Nebensache. In den Baumknoten kannst du ja immer noch rohe Zeiger benutzen.

volkard

seldon schrieb:

Warum nicht
struct bin_tree {
    std::vector<std::unique_ptr<node>> nodes;
}
? Oder meinethalben auch einen boost::ptr_vector, das ist dann Nebensache. In den Baumknoten kannst du ja immer noch rohe Zeiger benutzen.

Das ist einfach nur Panne.
Hoffe, die C++-Gemeinde findet irgendwann wieder den Weg zurück.

knivil schrieb:

Dem Dogma folgend koennen alle rohen Zeiger durch unique_ptr ersetzt werden und man spart sich die eigenen Aufrufe von delete

Von welchem Dogma sprichst du genau? Zumindest der RAII-Artikel sagt ja explizit, dass solche Fälle von new/delete legitim sind. Aber das kapselst du gegen aussen auch schön weg. Problematisch ist, wenn sich der Anwender ständig mit manueller Speicherverwaltung rumschlagen muss. nicht der Implementierer der Datenstruktur ein einziges Mal.

knivil schrieb:

Auf Ebene der Knoten ist es manuelles Speichermanagment, auf Ebene des Benutzers, der nur bin_tree zu Gesicht bekommt, ist es RAII.

Genau so ist es ja gut... Sind wir uns nun einig oder wie?

volkard schrieb:

Hoffe, die C++-Gemeinde findet irgendwann wieder den Weg zurück.

Wohin? C? Assembler?

volkard

manni66 schrieb:

volkard schrieb:

Hoffe, die C++-Gemeinde findet irgendwann wieder den Weg zurück.

Wohin? C? Assembler?

C++

volkard schrieb:

manni66 schrieb:

volkard schrieb:

Hoffe, die C++-Gemeinde findet irgendwann wieder den Weg zurück.

Wohin? C? Assembler?

C++

Und das ist, wenn man delete schreibt?

volkard

manni66 schrieb:

volkard schrieb:

manni66 schrieb:

volkard schrieb:

Hoffe, die C++-Gemeinde findet irgendwann wieder den Weg zurück.

Wohin? C? Assembler?

C++

Und das ist, wenn man delete schreibt?

Unfug. Warum antworte ich eigentlich noch auf unregs?

Zwischenfrager schrieb:

knivil schrieb:

Dem Dogma folgend koennen alle rohen Zeiger durch unique_ptr ersetzt werden und man spart sich die eigenen Aufrufe von delete

Von welchem Dogma sprichst du genau? Zumindest der RAII-Artikel sagt ja explizit, dass solche Fälle von new/delete legitim sind. Aber das kapselst du gegen aussen auch schön weg. Problematisch ist, wenn sich der Anwender ständig mit manueller Speicherverwaltung rumschlagen muss. nicht der Implementierer der Datenstruktur ein einziges Mal.

knivil schrieb:

Auf Ebene der Knoten ist es manuelles Speichermanagment, auf Ebene des Benutzers, der nur bin_tree zu Gesicht bekommt, ist es RAII.

Genau so ist es ja gut... Sind wir uns nun einig oder wie?

volkard schrieb:

seldon schrieb:
Warum nicht
struct bin_tree {
    std::vector<std::unique_ptr<node>> nodes;
}
? Oder meinethalben auch einen boost::ptr_vector, das ist dann Nebensache. In den Baumknoten kannst du ja immer noch rohe Zeiger benutzen.
Das ist einfach nur Panne.
Hoffe, die C++-Gemeinde findet irgendwann wieder den Weg zurück.

Das sehe ich anders. Ich kann, da du keine Begründung angegeben hast, über deine Beweggründe nur spekulieren; im Vergleich zum Code mit nackten Zeigern ergibt sich (vom kürzeren Code abgesehen) vor allem ein Unterschied für die Schwierigkeit, in Konstruktoren von bin_tree Exceptionsicherheit zu garantieren.

Stell dir vor, einen Kopierkonstruktor für bin_tree zu schreiben:

bin_tree::bin_tree(bin_tree const &other) {
  for(auto &p : other.nodes) {
    nodes.push_back(new node(*p)); // oder p->clone() oder wasauchimmer.
  }
}

...geht mit unique_ptr wunderbar. Mit nackten Zeigern ist der Code fehlerhaft, und das ist ein einfach zu machender Flüchtigkeitsfehler.

bin_tree::bin_tree(bin_tree const &other) {
  try {
    for(auto p : other.nodes) {
      nodes.push_back(new node(*p)); // oder p->clone() oder wasauchimmer.
    }
  } catch(...) {
    for(auto p : other.nodes) {
      delete p;
    }
    throw;
  }
}

ist für mein Auge weniger hübsch.

knivil

Kopierkonstruktor ist anwendungsbedingt, d.h. ich will meine Baeume/Graphen meist nicht kopieren, da es recht kompliziert ist, die Verknuepfung zwischen Knoten zu kopieren. Auch gibt es einfachere Varianten, um Probleme zu vermeiden:

bin_tree::bin_tree(bin_tree const &other) { 
   for(auto p : other.nodes) {
     std::unique_ptr<node> pnode(new node(p)); // schwierig, dass auf Knotenebene zu machen
                                               // weil die Nachbarknoten in der Kopie noch nicht existieren koennten
     nodes.push_back(pnode.get());
     pnode.release();
   } 
}

Nur so am Rande: Wahrscheinlich wuerde ich hier doch eine Liste benutzen.

Zur Frage:

Warum nicht
struct bin_tree {
    std::vector<std::unique_ptr<node>> nodes;
}
? Oder meinethalben auch einen boost::ptr_vector, das ist dann Nebensache. In den Baumknoten kannst du ja immer noch rohe Zeiger benutzen.

Smart pointer werden bei mir eingesetzt, wenn Besitz transferiert wird, meist an Modul- oder Klassengrenzen. Das ist hier nicht der Fall ( struct node kann wegen mir innerhalb von struct bin_tree definiert werden). D.h. Besitzverhaeltnisse auszudruecken ist nicht noetig. Weiterhin stelle ich mir immer die Frage, wieviel Abstraktion ich gerne haette, was es bringt und was es kostet. Mit unique_ptr fuegt man ein weiteres Level hinzu, kostet wenig (nicht im Sinne von Leistung, aber vielleicht Lesbarkeit), bringt aber auch wenig. Deswegen benutze ich es nicht. Zu guter letzt: konform zu C++03, was leider immer noch wichtig ist.

hustbaer

Ich sehe jetzt auch kein Problem mit ptr_vector in dem beschriebenen Fall.
Oder eben nen Pool-Allocator verwenden.
Beides garantiert Exception-Sicherheit im Ctor.

Ich mag nämlich genau so wenig wie seldon "halb" Exception-sicheren Code.

Natürlich kann man dafür auch den vector in eine Basisklasse auslagern. Das wäre auch OK.
Wobei man, wenn man diese Variante konsequent weiterdenkt, irgendwann wieder beim ptr_vector landet. Also wieso nicht gleich den verwenden?

hustbaer

knivil schrieb:

Auch gibt es einfachere Varianten, um Probleme zu vermeiden:

bin_tree::bin_tree(bin_tree const &other) { 
   for(auto p : other.nodes) {
     std::unique_ptr<node> pnode(new node(p)); // schwierig, dass auf Knotenebene zu machen
                                               // weil die Nachbarknoten in der Kopie noch nicht existieren koennten
     nodes.push_back(pnode.get());
     pnode.release();
   } 
}

Das reicht noch nicht.
Was wenn es bei der zweiten Node nen bad_alloc gibt? Wer löscht dann die erste?

Davon abgesehen musst du dieses unique_ptr/release Spiel überall wiederholen wo du Nodes einfügst - auch nicht schön.

Eine Lösung für das Freigeben der ersten Node hab' ich ja schon geschrieben (Basisklasse).
Und für das Einfügen würde ich eher ne Hilfsfunktion verwenden:

template <class... T>
bin_tree::node* bin_tree::alloc_node(T&&... args)
{
    std::unique_ptr<node> p(new node(std::forward<T>(args)...));
    nodes.push_back(p);
    return p.release();
}

bin_tree::bin_tree(bin_tree const &other)
{ 
    for (auto p : other.nodes)
        alloc_node(p);
}

Und wenn man das jetzt konsequent weiterdenkt, dann landet man irgendwann bei nem Pool-Allocator.

Wobei man jetzt diesen Pool-Allocator nicht unbedingt voll aufblasen muss, so dass er alles kann was er können könnte, breitmöglichst einsetzbar ist etc.
Lieber den Ball flach halten, und nur das implementieren was man braucht.

Trotzdem finde ich es gut das Single-Responsibility Prinzip einzuhalten.
Und das hiesse für mich, dass die "sicher newen und wieder deleten" Logik in eine eigene Klasse gehört.

Und dann sind wir wieder da wo wir (mMn.) hingehören: es gibt nur mehr RAII-verwendende Klassen und "RAII-umsetzende" Klassen, und die "RAII-Umsetzert" tun wenig bis nichts anderes.

TyRoXx

seldon schrieb:

...geht mit unique_ptr wunderbar. Mit nackten Zeigern ist der Code fehlerhaft, und das ist ein einfach zu machender Flüchtigkeitsfehler.
bin_tree::bin_tree(bin_tree const &other) {
  try {
    for(auto p : other.nodes) {
      nodes.push_back(new node(*p)); // oder p->clone() oder wasauchimmer.
    }
  } catch(...) {
    for(auto p : other.nodes) {
      delete p;
    }
    throw;
  }
}
ist für mein Auge weniger hübsch.

Da ist immer noch ein Leck. Vielleicht ist das hier korrekt:

bin_tree::bin_tree(bin_tree const &other) {
  try {
    for(auto p : other.nodes) {
	  auto p2 = new node(*p);
	  try {
        nodes.push_back(p2);
	  }
	  catch(...) {
	    delete p2;
	    throw;
	  }
    }
  } catch(...) {
    for(auto p : other.nodes) {
      delete p;
    }
    throw;
  }
}

Hmm, ist irgendwie voll schwierig Exception-sicher zu programmieren. Wenn C++ dafür bloß eine Lösung hätte...

hustbaer schrieb:

Eine Lösung für das Freigeben der ersten Node hab' ich ja schon geschrieben (Basisklasse).

Wow such OOP. Genau richtig für knivil.

knivil

Ich kehre zurueck zu meiner Loesung: Kein Kopierkonstruktor. Warum jetzt kuenstlich einen dazubasteln, wenn es von Anfang an nicht vorgesehen war. Wahrscheinlich haette ich dann mit Indizes gearbeitet anstatt mit Zeigern, vielleicht auch nicht. Dann waers ein einfaches copy , vielleicht auch nicht.

Das Hauptproblem ist, dass kein Destruktor aufgerufen wird, wenn im Konstruktor eine Exception geworfen wird. Das kann aber per Hand geschehen:

bin_tree::bin_tree(bin_tree const &other) {
   nodes.resize(other.nodes.resize(),0)
   for(unsigned int i = 0; i < nodes.size(); ++i) {
       nodes[i] = new (nothrow) node(other.nodes[i]); // whatever that means in this context
       if (node[i] == 0) {
           this->bin_tree::~bin_tree();
           throw std::bad_alloc ...;
       }
   }
}

Nun, wuerd ich wahrscheinlich nicht so machen. Ist nicht zu Ende gedacht (e.g. virtual Destruktor). Aber ihr werdet mir hoffentlich gute Argumente aus dem Standard liefern. Und ja, man kann auch einfach ptr_container verwenden.

Ausserdem: Niemand nimmt sich dem Problem an: benachbarte Knoten sollen nah im Speicher liegen.

Wow such OOP. Genau richtig für knivil.

Just another Fan-Boy. Wer argumentiert hat verloren. Du hast gewonnen. Jaja, ich habe RAII nicht verstanden, OOP auch nicht, und ueberhaupt kann ich nicht programmieren, schon gar nicht C++.

hustbaer

knivil schrieb:

Ich kehre zurueck zu meiner Loesung: Kein Kopierkonstruktor.

Das ist natürlich OK. Wozu alles copyable wenn mans nicht braucht? Macht keinen Sinn. KISS FTW!

Das Hauptproblem ist, dass kein Destruktor aufgerufen wird, wenn im Konstruktor eine Exception geworfen wird. Das kann aber per Hand geschehen:
bin_tree::bin_tree(bin_tree const &other) {
   nodes.resize(other.nodes.resize(),0)
   for(unsigned int i = 0; i < nodes.size(); ++i) {
       nodes[i] = new (nothrow) node(other.nodes[i]); // whatever that means in this context
       if (node[i] == 0) {
           this->bin_tree::~bin_tree();
           throw std::bad_alloc ...;
       }
   }
}
Nun, wuerd ich wahrscheinlich nicht so machen.

Hoffentlich! Die Lösung ist nämlich auch grausam.
Wenn man sowas 1-2 mal in einem Programm braucht, OK. Nur kenne ich das aus eigenen Projekten. Wenn man erstmal anfängt mit "ist ja nur die eine Stelle" zu argumentieren, dann zieht sich das bald durchs ganze Projekt. *schauder*

Denn new (nothrow) garantiert ja soweit ich weiss nur dass der 1. operator new Aufruf keine Exception wirft. Der Node-Konstruktor könnte aber eine werfen. D.h. man verlässt sich darauf dass dieser noexcept ist. Finde ich nicht gut, zumindest nicht wenn es einfach zu vermeiden ist.

Was es in diesem Fall ja - wie ich schon mehrfach geschrieben habe - ist. Und wer jetzt die Nase rümpfen will weil hier Vererbung als Implementierungs-Detail eingesetzt wird, der soll das ruhig machen.

knivil schrieb:

Ausserdem: Niemand nimmt sich dem Problem an: benachbarte Knoten sollen nah im Speicher liegen.

Wenn du mir sagst wie man das mit manuellem delete hinbekommt, dann sag ich dir wie man es ohne macht (bzw. fange an darüber nachzudenken).
Oder hab' ich da was übersehen?

Wobei es hier natürlich zwei offensichtliche Lösungen mit RAII gibt:

std::vector<Node> + Index
* Pool-Allokatoren

Also was ist jetzt die Frage?

@TyRoXx
Nein, nix OOP, Implementierungsdetail. Die Hilfsklasse wird natürlich private geerbt.