Korrekte Benutzung von delete bei Verwendung von new aus dem Konstruktor

Steffo

@wob sagte in Korrekte Benutzung von delete bei Verwendung von new aus dem Konstruktor:

Die "Lösung" ist es, generell zu versuchen, rohe besitzende Zeiger zu vermeiden. Wenn du in deinem Verbund stattdessen std::unique_ptr<Rechteck> r1, r2; std::unique_ptr<Halbkreis> hk; verwendest, kannst du die deletes alle sparen. Beim ersten Konstruktor nimmst du ebenfalls std::unique_ptr als Parameter, um anzuzeigen, dass du den Besitz übernimmst. Oder du nutzt shared_ptr, wenn die Ownership geteilt sein soll. Oder du nimmst Rechteck ohne * als Variablen/Parameter und speicherst Kopien.

Eine weitere Variante wäre std::move().

wob

@Steffo sagte in [Korrekte Benutzung von delete bei Verwendung von new aus dem Konstruktor]

Eine weitere Variante wäre std::move().

?

Steffo

@wob sagte in Korrekte Benutzung von delete bei Verwendung von new aus dem Konstruktor:

@Steffo sagte in [Korrekte Benutzung von delete bei Verwendung von new aus dem Konstruktor]

Eine weitere Variante wäre std::move().

?

public:
    Verbund(Rechteck r1, Rechteck r2, Halbkreis h): r1(std::move(r1)) , r2(std::move(r2)), hk(std::move(h)){}

wob

Ach so, das meinst du. Da die Klassen hier eh nur 2 doubles haben oder so, sehe ich für move hier überhaupt keine Notwendigkeit. Das halte ich auch eher für ein Implementierungsdetail im Konstruktor. Ich denke, es ist hier viel wichtiger, erst einmal zu überlegen, welchen Typ r1, r2 und hk hier haben sollten.

Von der korrekten Nutzung her musst du nur aufpassen, dass du deinen Pointer entweder auf null setzt oder den Speicher tatsächlich allokierst, sodass im Destruktor semantisch gesehen einfach nur dein delete p; reichen wird. Aber heutzutage benutzt man eh die smarten Pointern.

Tamoko

Vielen Dank für die Tipps. Durch die Links werde ich mich mal durchlesen.
Ich hatte das Programm zuvor schon komplett ohne Verwendung von Pointern geschrieben und einfach Objekte auf dem Stack (also new/delete) verwendet und mit Kopien/Referenzen gearbeitet. Die Aufgabenstellung war mehr oder weniger zur Übung frei vorgegeben, es ging auch eher um die Benutzung von Vererbung und virtual. Ich wollte hier einfach mal testen, wie ich es am Besten mit Zeigern umsetze.

Zwecks der shared- bzw unique Pointer hab ich die Verbundklasse nochmal umgeschrieben, wäre sie so korrekt?

class Verbund: public Querschnitt
{
   unique_ptr<Rechteck> r1, r2;
   unique_ptr<Halbkreis> h;

public:
    Verbund(Rechteck *r1, Rechteck *r2, Halbkreis *h): r1(r1), r2(r2), h(h) {}

    Verbund(double b1, double t1, double b2, double t2, double r): r1(new Rechteck(b1, t1)), r2(new Rechteck(b2, t2)), h(new Halbkreis(r)){}

    ~Verbund() override {}

    double berechneFlaeche() override
    {
        return r1->berechneFlaeche() + r2->berechneFlaeche() + h->berechneFlaeche();
    }
};

wob

@Tamoko sagte in Korrekte Benutzung von delete bei Verwendung von new aus dem Konstruktor:

Verbund(Rechteck *r1, Rechteck *r2, Halbkreis *h): r1(r1), r2(r2), h(h) {}

Nein.

Lies bitte nochmal den Abschnitt in den Core Guidelines. Das Problem an dieser Zeile ist, dass man nicht erkennt, dass die Owership übertragen wird. Im Prinzip also die Frage: wer löscht?

Wenn ich Code habe wie:

auto r1 = new Rechteck();
auto r2 = new Rechteck();
auto hk = new Halbkreis();

{
  Verbund v(r1, r2, hk);
}

// Folgende Zeile ja oder nein:
delete r1; delete r2; delete hk;

Problem: du siehst dem Verbund nicht an, dass dieser r1, r2 und hk übernimmt und die folgenden deletes also falsch sind.

Daher ist normalerweise die Idee, dass du bei einem Rechteck* nicht die Ownership übernimmst.
Wenn dein Rechteck die Objekte übernehmen soll, dann:

Verbund(std::unique_ptr<Rechteck> r1, ...);

auto r1 = std::make_unique<Rechteck>();
...
Verbund v(std::move(r1), ...);
...

Hier wird dann explizit die Übertragung des Besitzes ausgedrückt.

Aber allgemein würde ich bei einem Rechteck wohl eher zu einer Kopie, also einem Nicht-Pointer, greifen.

Edit: GUI-Interfaces sind oft anders. Da wird gerne einem Fenster z.B. ein Button hinzugefügt und dann übernimmt das Fenster den Besitz. Diese Interfaces sind aber auch vor C++11 entstanden.

Jockelx

OT:
Was soll das override im Destruktor? Da wird nichts überschrieben. Meckert dein Kompiler da nicht?

wob

@Jockelx sagte in Korrekte Benutzung von delete bei Verwendung von new aus dem Konstruktor:

OT:
Was soll das override im Destruktor? Da wird nichts überschrieben. Meckert dein Kompiler da nicht?

Es stellt sicher, dass ~Querschnitt virtual ist, würde ich mal behaupten.

Zugegeben, die Vererbungshierachie ist hier merkwürdig.

@Jockelx sagte in Korrekte Benutzung von delete bei Verwendung von new aus dem Konstruktor:

OT:
Was soll das override im Destruktor? Da wird nichts überschrieben. Meckert dein Kompiler da nicht?

Ich würde da sogar fragen: was soll der leere Destruktor in den abgeleiteten Klassen? Einmal virtual ist immer virtual.

Jockelx

@wob sagte in Korrekte Benutzung von delete bei Verwendung von new aus dem Konstruktor:

Es stellt sicher, dass ~Querschnitt virtual ist, würde ich mal behaupten.

Oh, cool, das kannte ich gar nicht. Nehme alles zurück (wobei @manni66 natürlich Recht hat mit "wozu überhaupt").

Tamoko

Danke für die Rückmeldungen. Also der Compiler hat nicht gemeckert wegen dem Keyword override, ich habe jetzt aber alle Destruktoren bis auf den virtual Destruktor der Basisklasse entfernt.

Ich habe mich, da wir bislang weder Move-Konstruktoren, noch das Keyword auto benutzt haben und solche Übungsaufgaben leider immer recht schwammig formuliert werden, dafür entschieden es erstmal beim Arbeiten mit Kopien zu belassen und in den abgeleiteten Klassen nicht mit Pointern zu arbeiten.