Destruktoren- Klarrheit schaffen

DocShoe

Es gibt nur einen Destruktor pro Klasse. Ob du alle Zeiger deines Objektes löschen musst hängt davon ab, ob das Objekt diesen Zeiger auch besitzt. Als Faustregel (wurde ja auch schon erwähnt) sollte jedes Objekt, das in seinem Konstruktor dynamischen Speicher anfordert, als Besitzer gelten und damit auch für die korrekte Freigabe verantwortlich sein.

class ClassA
{
   ...
};

class ClassB
{
   // kein Besitz, nur Zeiger auf ein anderes Objekt vom Typ Class A
   ClassA* data1_;      

   // echter Besitz des dynamisch erzeugten Objektes
   ClassA* data2_;

public:
   ClassB( ClassA* d1 ) :
      data1_( d1 ),
      data2_( new ClassA() )

   ~ClassA()
   {
      delete data2_;
   }

private:
   // Kopien verbieten
   ClassB( const ClassB& other );
   ClassB& operator=( const ClassB& other );
};

int main()
{
   ClassA ca;
   ClassB cb( &ca );
}

In diesem Beispiel hat ClassB zwei Zeiger auf jeweils ein Objekt vom Typ ClassA, einer davon wird von aussen bereitgestellt, der andere innerhalb des Konstruktors dynamisch erzeugt. Das Objekt cb besitzt nur das Objekte, auf das data2_ zeigt und kann data1_ im Destruktor ignorieren. Für data2_ ist es aber verantwortlich und muss daher dafür sorgen, dass es im Destruktor durch einen Aufruf von delete korrekt abgeräumt wird.

Ok also alle zeiger für die ich innerhalb der Klasse "new" aufrufe, sollte ich in der Regel im Destruktor löschen?!

Nun kommt es aber noch etwas komplizierter:

class A;
class Class
{
private:
    string Name;
public:
    map<string, A*> testmap;
    A* pA;
    string na;
    void f()
    {
        na = "Nr.";
        for(i=0; i<5; i++)
        {
            na = na + "I"
            pA = new A();
            pA->Name = na;
            testmap.insert(pair<string, A*>(pA->eName, pA));
        }   
    }

    Class()
    {
         pA = new A();
         pA->Name = "test";
         testmap.insert(pair<string, A*>(pA->eName, pA));
    }
    ~Class();  //Inhalt???
};

DocShoe

Am Prinzip hat sich nichts geändert, nur dass du keine benannten Variablen mehr hast, sondern dynamisch erzeugte Objekte in einem Container speicherst. Die Klasse Class ist für das Löschen dieser Objekte verantwortlich und sollte das im Destruktor auch machen.

Braunstein

pA sollte hier keine Membervariable sein sondern lokal in f und im Konstruktor.
Dann könnte der Destruktor so aussehen.

~Class() {
    for(std::map<std::string, A*>::iterator it = testmap.begin(); it!=testmap.end(); ++it)
        delete it->second; 
}

Wieso muss dafür pA lokal sein? Kann ich den Destruktor nicht auch so verwenden?

Niemand hat gesagt muss. Da du die per pA referenzierten Objekte aber überhaupt nur im Konstruktor und f lokal verwendest, hättest du die A-Variable auch als lokale ausführen können (deshalb sprach mein Vorredner von "sollte").

Wenn du das nicht tust, dann muss du in f() noch den Speicherbereich, auf den pA bis dahin zeigt freigeben und ihm dann erst einen neuen zuweisen. Im Moment hast du dort ein Speicherleck.

Braunstein

Da ist kein Speicherleck wenn im Destruktor die map ordentlich abgeräumt wird.
pA als Member kann hier schlicht Verwirrung stiften. Dann könnte leicht der Fall eintreten, dass man ein Objekt versucht zweimal zu löschen.

Das kann ich nun nicht ganz nach vollziehen. Wieso muss ich den Speicherbereich, auf den pA vorher zeigt jedes mal freigeben? Jeder dieser Speicherbereiche wird doch noch in meiner map gebraucht/ verwaltet.

Ich habe auch versucht, pA lokal zu definieren. Aber leider scheint meine map, wenn ich sie dann irgendwo benutze, leer zu sein. Nur wenn pA global in der Klasse definiert ist, kann ich die map später benutzen...

Braunstein

Wie hast du dann pA lokal benutzt?

Ahh, siehste, ich hab nur bis zum pA->eName gelesen. Na, dann scheinst du es doch jetzt verstanden zu haben^^

edit: Wobei du pA natürlich lokal definieren könntest in den Funktionen, gibt ja keinen Grund diesen "temporär"-Zeiger als Zustand des Objekts zu betrachten, es sei denn, du brauchst immer einen Zeiger auf das zuletzt eingefügte Objekt!

Braunstein

Decimad schrieb:

edit: Wobei du pA natürlich lokal definieren könntest in den Funktionen, gibt ja keinen Grund diesen "temporär"-Zeiger als Zustand des Objekts zu betrachten, es sei denn, du brauchst immer einen Zeiger auf das zuletzt eingefügte Objekt!

Das sagte ich bereits.

Herrje, hier ging's halt gerade drüber und drunter... ich antwortete noch auf seine Frage bezüglich der Lokalität, aber hatte seinen Quelltext vorher mental falsch geparsed also habe ich meine Antwort bezüglich der Lokalität entsprechend geupdated, weil meine ursprüngliche Erklärung schlicht falsch war

So würde das ganze mit lokalem pA aussehen:

class A;
class Class
{
private:
    string Name;
public:
    map<string, A*> testmap;
    //A* pA;
    string na;
    void f()
    {
        na = "Nr.";
        for(i=0; i<5; i++)
        {
            na = na + "I";
            A* pA = new A();
            pA->Name = na;
            testmap.insert(pair<string, A*>(pA->eName, pA));
        }  
    }

    Class()
    {

         A* pA = new A();
         pA->Name = "test";
         testmap.insert(pair<string, A*>(pA->eName, pA));
    }
    ~Class();  //Inhalt???
};

Die map wird zwar gefüllt, aber sobald ich eine Funktion von A benutze wird an der Stelle abgebrochen. Als scheint es als würden die Speicherbereiche ungültig sein?!

Braunstein

Der Code ist soweit Ok. Der Speicher sollte nicht ungültig sein. Zeige bitte mal ein Minimalbeispiel wo der Fehler bei dir auftritt.

Eisflamme

Absolute_nooby schrieb:

So würde das ganze mit lokalem pA aussehen:

class A;
class Class
{
private:
    string Name;
public:
    map<string, A*> testmap;
    //A* pA;
    string na;
    void f()
    {
        na = "Nr.";
        for(i=0; i<5; i++)
        {
            na = na + "I";
            A* pA = new A();
            pA->Name = na;
            testmap.insert(pair<string, A*>(pA->eName, pA));
        }  
    }

    Class()
    {
        
         A* pA = new A();
         pA->Name = "test";
         testmap.insert(pair<string, A*>(pA->eName, pA));
    }
    ~Class();  //Inhalt???
};

Die map wird zwar gefüllt, aber sobald ich eine Funktion von A benutze wird an der Stelle abgebrochen. Als scheint es als würden die Speicherbereiche ungültig sein?!

Also der Code erscheint mir nicht kompilierfähig. A ist doch nur vorwärtsdeklariert, wie soll man da pA->Name nutzen können? Minimalbeispiel wie gesagt ftw.

padreigh

müll geschrieben

Ich weiß ehrlich nicht, was ich vorhin gemacht hatte, aber nachdem ich ein Beispiel geschrieben hatte und den Fall nicht reproduzieren konnte, habe ich nochmals versucht in meinem Programm pA lokal zu definieren. Nun klappts... Ich frage mich echt, was ich da gemacht hatte...

Trotzdem danke an alle!

hier noch das (funktionierende) Beispielprogramm bei Interesse:

#include <string>
#include <list>
#include <map>
#include <vector>

using namespace std;

class A
{
public:
	string Name;
	void meth()
	{

	}

};

#include <string>
#include <list>
#include <map>
#include <vector>

#include "A.h"

using namespace std;

class Klasse
{
private:
    string Name;
public:
    map<string, A*> testmap;
    //A* pA;
    string na;
    void f()
    {
        na = "Nr.";
        for(int i=0; i<5; i++)
        {
            na = na + "I";
            A* pA = new A();
            pA->Name = na;
            testmap.insert(pair<string, A*>(pA->Name, pA));
        }  
    }

    Klasse()
    {

         A* pA = new A();
         pA->Name = "test";
         testmap.insert(pair<string, A*>(pA->Name, pA));
    }
    ~Klasse()
	{
		for(std::map<std::string, A*>::iterator it = testmap.begin(); it!=testmap.end(); ++it)
        delete it->second; 
	}
};

int main
{
     Klasse test;
     test.f();
     std::map<std::string, A*>::iterator it = test.testmap.find("Nr.I");
     it->second->meth();
}

padreigh

using namespace std; in .h's ist EVIL
und den string na kannst du auch lokal in die f() packen ..

DocShoe

So, nachdem die Grundlagen erklärt sind kann man ja shared_ptr aus TR1 oder boost erwähnen. Oder fragen, warum die Objekte denn unbedingt auf dem Heap erzeugt werden müssen, statt dem Container die Speicherverwaltung zu überlassen.

Edit:
So ein Iterator muss nicht immer auf ein gültiges Element zeigen, find kann auch schon mal end() zurückgeben und dann sollte man den Iterator besser nicht dereferenzieren und Operationen ausführen.