Destruktoren- Klarrheit schaffen

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.

Eisflamme

Wenn A so simpel ist wie hier dargestellt, würde ich das aber ohne Zeiger in die Map packen. Und wenn man A nur in der Map hat, ist shared_ptr doch total übertrieben?

HighLigerBiMBam

Eisflamme schrieb:

Wenn A so simpel ist wie hier dargestellt, würde ich das aber ohne Zeiger in die Map packen. Und wenn man A nur in der Map hat, ist shared_ptr doch total übertrieben?

shared_ptr braucht man, wenn verschiedene Objekte auf das selbe zugreifen müssen. Wenn es allerdings ein alleiniger Besitz sein soll reicht, scoped_ptr oder auto_ptr mehr als aus.

ps: Google mag auto_ptr nicht :p

Google schrieb:

If you actually need pointer semantics, scoped_ptr is great. You should only use std::tr1::shared_ptr under very specific conditions, such as when objects need to be held by STL containers. You should never use auto_ptr.