Korrekte Benutzung von delete bei Verwendung von new aus dem Konstruktor



  • Ich habe eine Frage zur Nutzung von Objektpointern und zwar wenn ich als Membervariablen einen oder mehrere Pointer auf Objekte anderer Klassen/Structs nutzen möchte. Wenn ich diese bei der Instanzierung von Objekten aus dem Konstruktor heraus verwende und dort mit new Speicher allokiere (siehe Beispiel Zeile 48-51)
    wo setze ich das delete um diesen Speicher wieder freizugeben oder macht das der Destruktor automatisch. Man hat uns hierzu erklärt, dass für jedes new auch ein delete gesetzt werden muss.

    #include <iostream>
    
    
    static const double PI = 3.14159f;
    
    class Querschnitt
    {
    public:
        virtual ~Querschnitt(){}
        virtual double berechneFlaeche() = 0;
    };
    
    
    class Rechteck: public Querschnitt
    {
        double b, t;
    public:
        Rechteck(float b, float t): b(b), t(t) {}
        ~Rechteck() override {}
        double berechneFlaeche() override
        {
            return b*t;
        }
    };
    
    class Halbkreis: public Querschnitt
    {
        double r;
    public:
        Halbkreis(double r): r(r){}
        ~Halbkreis() override {}
    
        double berechneFlaeche() override
        {
            return (PI * r * r)/2;
        }
    };
    
    class Verbund: public Querschnitt
    {
        Rechteck *r1 = nullptr;
        Rechteck *r2 = nullptr;
        Halbkreis *hk = nullptr;
    public:
        Verbund(Rechteck *r1, Rechteck *r2, Halbkreis *h): r1(r1) , r2(r2), hk(h){}
        Verbund(double b1, double t1, double b2, double t2, double h)
        {
            /* Wo muss das delete hierfür stehen? */
            r1 = new Rechteck(b1, t1);
            r2 = new Rechteck(b2, t2);
            hk = new Halbkreis(h);
        }
    
        ~Verbund() override {}
    
        double berechneFlaeche() override
        {
            return r1->berechneFlaeche() + r2->berechneFlaeche() + hk->berechneFlaeche();
        }
    };
    
    int main(){
    
        Querschnitt  *querschnittptr [3] {new Rechteck(1, 2), new Halbkreis(2), new Rechteck(2, 5)};
    
        for(Querschnitt *q: querschnittptr)
        {
            std::cout << q->berechneFlaeche() << std::endl;
        }
    
        Querschnitt *v = new Verbund(1, 2, 3, 1, 2);
        std::cout << "Flaeche von *v: " << v->berechneFlaeche() << std::endl;
        delete v;
        for(Querschnitt *q: querschnittptr)
            delete q;
    }
    
    

    Muss ich explizit wie folgt in Zeile 54 löschen?

    ~Verbund() override {
            delete r1;
            delete r2;
            delete hk;
        }
    

    Und falls ja, führt das nicht zum Absturz da der andere Konstruktor in Zeile 45 kein new verwendet. Ich bin da etwas verwirrt wie ich das sauber umsetze ohne dass der Speicher nicht mehr vollständig freigegeben wird. Wäre schön wenn mir jemand das erklären könnte!
    Und generell: Ist es sinnvoll das new überhaupt innerhalb des Konstruktors zu verwenden oder erzeugt man die Objekte lieber aus der main-Methode (sonst extern) und übergibt dem Konstruktor dann einfach die nötigen Speicheradressen?



  • Nach jedem new folgt ein delete, das ist schon richtig.
    Auch der Destruktor ist soweit ok, wenn du wirklich sicherstellen kannst, dass bei Zeile 45 im Heap alloziierte Objekte übergeben werden und der Aufrufer diese nicht ebenfalls löscht. Wenn du der Nutzer dieser Klasse bist, kannst du das sicherstellen, in dem du einfach die Klasse Verbund richtig benutzt.

    Aber davon mal abgesehen: Musst du hier wirklich ein new machen? Wie wäre es damit die Objekte auf dem Stack anzulegen und diese per Referenzen zu übergeben? Dann kannst du dir die Speicherverwaltung sparen.



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

    Und falls ja, führt das nicht zum Absturz da der andere Konstruktor in Zeile 45 kein new verwendet. Ich bin da etwas verwirrt wie ich das sauber umsetze ohne dass der Speicher nicht mehr vollständig freigegeben wird.

    In der Tat, da hast du ein Problem richtig erkannt. Wenn der andere Konstruktor benutzt wird, muss sichergestellt sein, dass die übergebenen Objekte von dir gelöscht werden dürfen. Das sieht man nicht direkt im Konstruktor-Interface und das ist somit problematisch.

    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.

    Bedenke auch, dass du, wenn du manuell new/delete nutzt, immer die Rule of 3 (bzw. Rule of 5) beachten musst, d.h. du musst irgendwas mit deinem Copy-Constructor machen. Wenn möglich, ist es daher besser, kein new/delete zu nutzen und stattdessen die Rule of 0 zu benutzen. Siehe z.B. hier: https://www.modernescpp.com/index.php/c-core-guidelines-constructors-assignments-and-desctructors

    Vielleicht liest du auch: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#S-resource



  • @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().



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

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

    ?



  • @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)){}
    


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



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


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



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



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



  • @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").



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


Log in to reply