[Anfängerproblem] Zeiger/Referenzen und Gültigkeitsbereiche

Ich schreibe ein Programm, das Atome und Moleküle verwaltet.

EIn Atom kann mit einem anderen Atom chemisch verbunden sein. Dafür möchte ich mir die Adresse des anderen Atoms abspeichern.
Grober Aufbau der Klasse.

class Atom
{
protected:
	double x;
	double y;
	double z;

	std::list<Atom* > neighbours;

public:
	Atom();
	virtual ~Atom();
	virtual void connect(Atom& atom);
	virtual void disconnect(Atom& atom);

};

// cpp - File
Atom::~Atom()
{
	for(std::list<Atom*>::iterator iter = neighbours.begin(); iter != neighbours.end(); iter++)
	{
	    disconnect(**iter);
	}
}
void Atom::connect(Atom& atom)
{
    this->neighbours.push_back(&atom);
    atom.neighbours.push_back(this);
}

void Atom::disconnect(Atom& atom)
{
    this->neighbours.remove(&atom);
    atom.neighbours.remove(this);
}

Ein Molekül besteht aus einem Vector von Atomen

class Molecule
{
protected:
	std::vector<Atom> atoms;
	void addAtom(const Atom&);
};

void Molecule::addAtom(const Atom& atom)
{
	atoms.push_back(atom);
}

Für Testzwecke möchte ich mir nun in einer Funktion zwei Testmoleküle erzeugen

createMolecule(string path)
{
	Molecule m;
	if (path == "")
	{
		Atom a;
		a.setPosition3d(1,0,0);
		Atom b;
		b.setPosition3d(0,1,0);

		//->>>>>>>>  
                a.connect(b);

		m.addAtom(a);
		m.addAtom(b);
	}
	else
	{
		Atom a;
		a.setPosition3d(2,3,0);
		Atom b;
		b.setPosition3d(1,1,0);

		m.addAtom(a);
		m.addAtom(b);
	}
	return m;
}

int main()
{
 Molecule a = createMolecule("");
 Molecule b = createMolecule("irgendwas");
 // tu was mit a und b---
 return 0;
}

Wenn ich das ausführe bekomme ich ganz einfach Speicherzugriffsfehler.
Das liegt höchstwahrscheinlich an den Zeigern, die ich speichere.
Ich sehe im Moment aber noch nicht die ganze Ursache, vermute aber irgendwelche
Kopierereien und Zeiger auf nicht-mehr-gültige Objekte.

Kann mir jmd sagen, was ich falsch mache und wie ich das besser hinbekomme?

Eine konkrete Frage wäre auch, ob std::list<Type>.push_back( const Referenz auf Type) eine Kopie einfügt oder nicht.

pyhax

In createMolecule() fügst du einen Zeiger auf das LOKALE Atom a und b ein. Nach dem verlassen dieser Funktion ist der Zeiger ungültig. Warum benutzt du Zeiger?

DocShoe

Das Problem liegt darin, dass du die Adresse von Elementen in einem Vektor benutzt. Wenn sich die Größe des Vektors ändert wird neuer Speicher allokiert und damit stimmen deine bisherigen Zeiger nicht mehr. Dazu gibt es mehrere Lösungsansätze:

Globale Verwaltung von Atomen und Molekülen, die dynamisch erzeugt und in einem Pool erzeugt werden. Der Pool ist hinterher für die korrekte Freigabe der Objekte verantwortlich.
Identifikation der Atome und Moleküle per ID, statt Zeiger merkst du dir nur die ID der Atome/Moleküle. Wo sie sich im Speicher befinden ist dann egal, du brauchst nur wieder einen Pool, der dir die passenden Objekte zu einer ID zurückgibt.
Verwendung von smart_ptr. smart_ptr benutzen einen Referenzzähler und geben die Pointees automatisch frei, wenn der Referenzzähler 0 erreicht.

Verwendung von smart_ptr. smart_ptr benutzen einen Referenzzähler und geben die Pointees automatisch frei, wenn der Referenzzähler 0 erreicht.

Mir käme jetzt nur shared_ptr in den Sinn. Allerdings weiß ich nicht ob das dem entspricht was ich möchte.

Solche smart Pointer habe ich bisher nur für mit new erzeugte Objekte gesehen.
Für mich wäre allerdings wichtig, dass die Zeiger auch auf andere Objekte zeigen können und kein delete aufgerufen wird.

{
  Atom a;
  Atom b;
  a.connect(b);
  // a sollte nun die adresse von b kennen und umgekehrt
}

Identifikation der Atome und Moleküle per ID, statt Zeiger merkst du dir nur die ID der Atome/Moleküle. Wo sie sich im Speicher befinden ist dann egal, du brauchst nur wieder einen Pool, der dir die passenden Objekte zu einer ID zurückgibt.

Das hatte ich auch schon im Sinn, allerdings finde ich es recht mühsam und umständlich.

Sollte das ganze eventuell funktionieren, wenn ich noch den Copy-Ctr und Zuweisungsoperator überschreibe, so dass beim Umkopieren automatisch die neuen Adressen verwendet werden und im Zuge des DeCtr die Disconnects aufgerufen werden?

DocShoe

Ich dachte dabei eher an sowas:

class Pool
{
   std::vector<Atom*> Atoms_;

public:
   ~Pool()
   {
      for( std::vector<Atom*>::iterator it = Atoms_.begin(); it != Atoms_.end(); ++it )
      {
         delete **it;
      }
   }

   Atom& acquire_atom()
   {
      Atoms_.push_back( new Atom() );
      return *Atoms_.back();
   }
};

int main()
{
   Pool p;
   Atom& a = p.acquire_atom();
   Atom& b = p.acquire_atom();

   a.connect( b );
}

Kannst dir jetzt natürlich noch was einfallen lassen, um nicht mehr benötigte Atome zu recyclen. Alle Atome haben jetzt die Lebenszeit des Pools, und da sie auf dem Heap erzeugt werden ändert sich ihre Adresse auch nie. Damit kannst du gefahrlos Zeiger benutzen.

314159265358979

Das ganze noch mit unique_ptr kombinieren und die Lösung ist nochmal besser

DocShoe

Habe hier kein C++11, kenn mich mit dem neumodischen Krams nicht aus

PS:
War das so eine Art Lob? Von Pi?

314159265358979

class Pool
{
   std::vector<std::unique_ptr<Atom>> Atoms_;

public:
   Atom& acquire_atom()
   {
      Atoms_.push_back(std::unique_ptr<Atom>(new Atom()));
      return *Atoms_.back();
   }
};

int main()
{
   Pool p;
   Atom& a = p.acquire_atom();
   Atom& b = p.acquire_atom();

   a.connect( b );
}

Mal unique_ptr reingesteckt. Hat zusätzlich noch den Vorteil, dass der Compiler keinen in diesem Fall gefärlichen Copy Ctor und Copy operator = generiert.

Das war eine Art "Noch bevor ich deinen Post gelesen habe, habe ich daran gedacht, es auch so vorzuschlagen.".

Nochmal für den nicht ganz so Weisen. Ich habe Kopier-Konstruktor und Zuweisungskonstruktor hinzugefügt, so wie ich es mir mal vorgestellt habe.
Das klappt immer noch nicht, aber ich weiß nicht an welcher Stelle mein Code genau crasht.
Ändert sich ein Vector, ruft er ja für jedes Element den Kopierkonstruktor auf.
In dem Kopierkonstruktor sollte mein meues Element ja dann die neue Adresse kennen, oder?

Atom::Atom()
{
    // TODO Auto-generated constructor stub

}

Atom::Atom(const Atom& other)
{
    std::cout << "KOPIERKTR" << std::endl;
    x = other.x;
    y = other.y;
    z = other.z;

    for (std::list<Atom*>::const_iterator iter = other.neighbours.begin();
            iter != other.neighbours.end(); iter++)
    {
        connect(**iter);
    }
}

Atom& Atom::operator=(const Atom& other)
{
    if (this != &other)
    {
        x = other.x;
        y = other.y;
        z = other.z;

        for (std::list<Atom*>::const_iterator iter = other.neighbours.begin();
                iter != other.neighbours.end(); iter++)
        {
            connect(**iter);
        }
    }
    return *this;
}

Atom::~Atom()
{
    for (std::list<Atom*>::iterator iter = neighbours.begin();
            iter != neighbours.end(); iter++)
    {
        disconnect(**iter);
    }
}

Ich habe versucht mir das mal aufzumalen:

Atom A , Adresse 1
Atom B , Adresse 2

connect (A,B )=>  A<2>  , B<1>.

Nun kommt ein Kopiervorgang:
A´ = Kopie von A.
A´ , Adresse 3
durch Kopierkonstruktor A´<2>  übernimmt die alte Adresse von B, B<1,3>
B hat nun das alte und neue A

A wird nun gelöscht:
B<3> hat nun nur noch Adresse von A´

B´ = Kopie(B) 
B´ Adresse 4
B´<3> kennt Adresse von neuem A´
A´<2,4>

B wird gelöscht:
A´ hat nun nur noch B´  <4>

und alle sollten glücklich sein...
zumindest nach dem Märchen

Wäre nett , wenn mir jmd meinem Denkfehler aufzeigen kann.

Falls ich nun doch eine Pool-Implementierung brauche, kann ich damit auch verschiedene Unterklassen von Atom problemfrei speichern?