Frage an Experten - Endlosschleife

Student83

Hi, seit einem Monat versuche ich nun den Fehler in folgendem Code zu finden, leider vergeblich was mich ziemlich wurmt. Und zwar habe ich als Übung eine doppelt verkettet Liste "List" geschrieben. Verwenden tut diese Liste Knoten vom Typ "ListNode". Iterieren kann man mittels der "ListIterator"-Klasse über die Liste.
Weiter hat meine Anwendung noch eine "Node"-Klasse die weitere Child-Nodes nun in dieser selbstgeschriebenen Liste ablegen soll. Das klappt soweit gut. Wenn ich (wie in der main-Funktion) nun aber hingehe und den Root-Node der Baumstruktur von Nodes mittels der Copy-Funktion kopiere und ausgebe hängt das Programm in einer Endlosschleife. Wer kann mir erklären warum das so ist?

Achso, der folgende Code ist nur schnell runtergeschrieben und auf das nötigste reduziert. Zeigt aber ganz gut wo das Problem ist.

#include <iostream>
#include <string>

template <class T>
class ListNode
{
public:
	ListNode();
	ListNode(const ListNode& listNode);

	ListNode* next;
	ListNode* previous;
	T value;
};

template <class T>
ListNode<T>::ListNode() :
	value()
{
	next = previous = nullptr;
}

template <class T>
ListNode<T>::ListNode(const ListNode& listNode)
{
	previous = listNode.previous;
	next = listNode.next;
	value = listNode.value;
}

//------------------------------------
//------------------------------------
//------------------------------------

template <class T>
class ListIterator
{
public:
	ListIterator();
	ListIterator(const ListNode<T>* node);

	ListIterator& operator++();
	const T& operator*() const;

	const ListNode<T>* node;
};

template <class T>
ListIterator<T>::ListIterator()
{
	node = nullptr;
}

template <class T>
ListIterator<T>::ListIterator(const ListNode<T>* node)
{
	this->node = node;
}

template <class T>
ListIterator<T>& ListIterator<T>::operator++()
{
	node = node->next;
	return *this;
}

template <class T>
const T& ListIterator<T>::operator*() const
{
	return node->value;
}

template <class T>
bool operator!=(const ListIterator<T>& lhs, const ListIterator<T>& rhs)
{
	return lhs.node != rhs.node;
}

//------------------------------------
//------------------------------------
//------------------------------------

template <class T>
class List
{
public:
	List();

	void Add(const T& value);
	ListIterator<T> Begin() const;
	ListIterator<T> End() const;

	int size;
	ListNode<T> root;
};

template <class T>
List<T>::List()
{
	size = 0;
	root.previous = root.next = &root;
}

template <class T>
void List<T>::Add(const T& value)
{
	ListNode<T>* newNode = new ListNode<T>();
	newNode->value = value;
	newNode->next = &root;
	newNode->previous = root.previous;
	root.previous->next = newNode;
	root.previous = newNode;
	size++;
}

template <class T>
ListIterator<T> List<T>::Begin() const
{
	return ListIterator<T>(root.next);
}

template <class T>
ListIterator<T> List<T>::End() const
{
	return ListIterator<T>(&root);
}

//------------------------------------
//------------------------------------
//------------------------------------

class Node
{
public:
	Node();
	Node(const Node& node);
	~Node();

	Node* Copy() const;
	void Append(Node* node);
	//Node& operator=(const Node& node);

	List<Node*> nodes;
	std::wstring text;
};

Node::Node()
{
}

Node::Node(const Node& node)
{
	*this = *node.Copy();
}

Node::~Node()
{
	ListIterator<Node*> iterator;
	for (iterator = nodes.Begin(); iterator != nodes.End(); ++iterator)
	{
		delete *iterator;
	}
}

Node* Node::Copy() const
{
	Node* node = new Node();
	ListIterator<Node*> iterator;
	for (iterator = nodes.Begin(); iterator != nodes.End(); ++iterator)
	{
		const Node* current = *iterator;
		Node* newNode = new Node(*current);
		node->nodes.Add(newNode);
	}
	node->text = text;
	return node;
}

void Node::Append(Node* node)
{
	nodes.Add(node);
}

/*Node& Node::operator=(const Node& node)
{
	if(this != &node)
	{
		Node tmp(node);
		std::swap(*this, tmp);
	}
	return *this;
}*/

//------------------------------------
//------------------------------------
//------------------------------------

void Print(const Node* node)
{
	if (node)
	{
		std::wcout << node->text << std::endl;
		ListIterator<Node*> iterator;
		int counter = 0;
		for (iterator = node->nodes.Begin(); iterator != node->nodes.End(); ++iterator)
		{
			counter++;
			std::wcout << counter << std::endl;
			const Node* current = *iterator;
			Print(current);
		}
	}
}

int main()
{
	Node* root = new Node();
	Node* sub = new Node();
	Node* subsub1 = new Node();
	Node* subsub2 = new Node();
	Node* subsub3 = new Node();
	Node* subsub4 = new Node();
	Node* subsub5 = new Node();

	root->text = L"root";
	sub->text = L"sub";
	subsub1->text = L"subsub1";
	subsub2->text = L"subsub2";
	subsub3->text = L"subsub3";
	subsub4->text = L"subsub4";
	subsub5->text = L"subsub5";

	root->Append(sub);
	sub->Append(subsub1);
	sub->Append(subsub2);
	sub->Append(subsub3);
	sub->Append(subsub4);
	sub->Append(subsub5);

	// Das geht!
	//Print(root);

	//Das geht nicht (Endlosschleife in Print-Funktion)!
	Node* node = root->Copy();
	Print(node);
	delete node;
	delete root;

	while(true){};
 	return 0;
}

camper

Deine Konstruktoren sind fast alle kaputt.

Student83

Wie meinst du das? Weil ich die Variablen nicht immer in die Initialisierungsliste geschrieben habe?

*Q* 1

*this = *node.Copy();

node.Copy erzeugt ein neues objekt auf dem heap, dass niemehr freigegeben wird.

Edit:
übrigens zu endlosschleife:

Node& Node::operator=(const Node& node) 
 { 
     if(this != &node) 
     { 
         Node tmp(node); 
         std::swap(*this, tmp); //<--
     } 
     return *this; 
 }

swap ruft wieder operator= auf. Das führt zu einer unendlichen rekursion.
Eigentlich würde ich keine endlosscheleifen, sondern einen stack-overflow erwarten.

Student83

Q schrieb:

*this = *node.Copy();
node.Copy erzeugt ein neues objekt auf dem heap, dass niemehr freigegeben wird.

Richtig, hier wird ein Node auf dem Heap erzeugt, allerdings wüsste ich nicht warum dieser Node nicht wieder freigegeben werden sollte. Schließlich wird am Ende des Programms der root node der Node-Baumstruktur sowie der root node der Node-Baumstruktur-Kopie gelöscht. Im Destruktor der Node-Klasse werden dann auch alle ChildNodes sowie deren ChildNodes usw. gelöscht. Würde nur

...; node.Copy();

aufgerufen hättest du natürlich recht, dann würde das Objekt nicht mehr freigegeben. Aber wer macht auch sowas ;).

Q schrieb:

Edit:
übrigens zu endlosschleife:
Node& Node::operator=(const Node& node) 
 { 
     if(this != &node) 
     { 
         Node tmp(node); 
         std::swap(*this, tmp); //<--
     } 
     return *this; 
 }
swap ruft wieder operator= auf. Das führt zu einer unendlichen rekursion.
Eigentlich würde ich keine endlosscheleifen, sondern einen stack-overflow erwarten.

Das war nur zum testen. Darum ist der Code auch ausgeklammert. Das kann also nicht der Grund für die Endlosschleife sein.

Für weitere Tipps bin ich dankbar.

Ich schau mir deinen code grad an, und muss erstmal sagen ich versteh nicht, was du machst. Ich versuch da mal durchzusteigen wies funktionieren soll, und was da genau schief läuft, aber das erste was mit aufgefallen ist: Wenn du in die endlosschleife kommst, gilt node == node->next, d.h. dein ListIterator::operator++ (node = node->next) macht schlicht und einfach nichts mehr. Woher das kommt muss ich mir erst noch anschauen.

Nunja, ich hab vorallem deine "Node" Klasse zwar immer noch nicht verstanden, aber immerhin das Problem gefunden. Bzw mir is aufgefallen, das deine List keinen operator= hat, der wird aber aufgerufen und der standard-operator= liefert halt nur müll. Richtig geschrieben, schon gehts.

Dir fehlt aber noch copy-ctor bei der List. Und folgendes ist mit Verlaub meiner Meinung nach ne absolute Katastrophe:

Node::Node(const Node& node)
{
    *this = *node.Copy(); //hier ruft der compilergenerierte operator= für Node text = node.text und nodes=node.nodes (letzteres verursacht dein problem) auf. Zusätzlich wird aber auch node.Copy() aufgerufen.
}

Node* Node::Copy() const
{
    Node* node = new Node();
    ListIterator<Node*> iterator;
    for (iterator = nodes.Begin(); iterator != nodes.End(); ++iterator)
    {
        const Node* current = *iterator;
        Node* newNode = new Node(*current); //hier wird der copy-ctor von Node aufgerufen! (was wiederum node.Copy() aufruft) Ist wohl eher reiner zufall dass es nicht hier schon aussteigt. Übersichtlich is das auf jedenfall nicht.
        node->nodes.Add(newNode);
    }
    node->text = text;
    return node;
}

Fazit: Achte immer gleich auf korrekte copy-ctors und operator=, dann wär dir das nicht passiert (und wenn du sie nicht gleich schreiben willst, mach sie private, dann fliegts dir wenigstens beim compilen um dir Ohren)

Mein List::operator=, mit dem man keine Endlosschleife mehr hat.

template <class T> const List<T>& List<T>::operator=(const List<T>& other)
{
   root.previous = root.next = &root;
   ListIterator<Node*> iterator;
   for (iterator = other.Begin(); iterator != other.End(); ++iterator)
   {
      this->Add(*iterator);
   }
   return *this;
}

camper

Der Code sagt mir, dass du schon mal was von der Regel der Drei gehört hast, und auch weist, wie man Copy-ctor , copy-Zuweisung und den Destruktor geeignet implementieren kann. Falls du dich im Anschluss dazu entschließt, diese Regeln grundlos über Bord zu werfen, kann es nicht unsere Aufgabe sein, den Fehler zu suchen.

Zu node-copy wurde schon etwas gesagt, allerdings beginnt das Problem viel eher:

template <class T>
class ListNode
{
public:
    ListNode();
    ListNode(const ListNode& listNode);

    ListNode* next;
    ListNode* previous;
    T value;
};

template <class T>
ListNode<T>::ListNode() :
    value()
{
    next = previous = nullptr;
}

template <class T>
ListNode<T>::ListNode(const ListNode& listNode)
{
    previous = listNode.previous;
    next = listNode.next;
    value = listNode.value;
}

Abgesehen von der fehlenden Zuweisung und dem fehlenden Destruktor, die logisch folgen müssten, sind beide Konstruktoren vollkommen unbrauchbar.
1. In jedem Falle muss der Erzeuger des Nodes diese "nachinitialiseren" und die Zeiger richtig setzen. Das bedeutet umgekehrt, dass die Konstruktoren ihrem Zwck nicht gerecht werden.
2. Der Copy-Konstruktor erzeugt keine äquivalente Kopie. Das Kopieren der Zeiger aus dem Ursprungsobjekt ist vollkommen sinnlos, da der neue Knoten trotzdem nicht Teil der Liste wird. Das heißt, dass es gar keine äquivalente Kopien geben kann. Also sollte das Kopieren von Nodes überhaupt verboten sein. Und wären Copy-ctor und Copy-Zuweisung verboten, würde der Rest auch nicht kompilieren, insbesondere der Copy-ctor von Node (der schon kritisiert wurde) würde dem Compiler Probleme bereiten.

Gegenwärtig sind die Zuständigkeiten für die Initilisierung bei dir verteilt. Das ist immer schlecht.
Benutze entweder dumme ListNodes und lasse List die gesamte Arbeit machen, oder mach die ListNodes intelligent genug, so dass sie sich um den Zeigerkram selbst kümmern können.

Student83

kleiner Troll schrieb:

Bzw mir is aufgefallen, das deine List keinen operator= hat, der wird aber aufgerufen und der standard-operator= liefert halt nur müll. Richtig geschrieben, schon gehts.

Seltsam, aber das erklärt schonmal einiges, denn eigentlich sollte in diesem Code nie eine Liste kopiert werden. Ich wollte von einem Node lediglich die Text-Variable und von dessen ChildNodes wiederum Kopien anfertigen die dann in die noch jungfräuliche Liste des kopierten Nodes eingefügt werden. Darum habe ich den Copy und Zuweisungsoperator auch noch nicht implementiert gehabt. Das eigentliche Problem ist also nicht die Liste, sondern wie du im folgenden geschrieben hast die Node-Klasse wo Murks passiert:

kleiner Troll schrieb:

Dir fehlt aber noch copy-ctor bei der List. Und folgendes ist mit Verlaub meiner Meinung nach ne absolute Katastrophe:

Node::Node(const Node& node)
{
    *this = *node.Copy(); //hier ruft der compilergenerierte operator= für Node text = node.text und nodes=node.nodes (letzteres verursacht dein problem) auf. Zusätzlich wird aber auch node.Copy() aufgerufen.
}

Node* Node::Copy() const
{
    Node* node = new Node();
    ListIterator<Node*> iterator;
    for (iterator = nodes.Begin(); iterator != nodes.End(); ++iterator)
    {
        const Node* current = *iterator;
        Node* newNode = new Node(*current); //hier wird der copy-ctor von Node aufgerufen! (was wiederum node.Copy() aufruft) Ist wohl eher reiner zufall dass es nicht hier schon aussteigt. Übersichtlich is das auf jedenfall nicht.
        node->nodes.Add(newNode);
    }
    node->text = text;
    return node;
}

Ok, jetzt bin ich ein wenig verwirrt. Ich möchte ja eine tiefe Kopie anfertigen. Wie müsste denn jetzt hier der Copy-Konstruktor und der Zuweisungsoperator korrekterweise auschauen?

kleiner Troll schrieb:

Mein List::operator=, mit dem man keine Endlosschleife mehr hat.

template <class T> const List<T>& List<T>::operator=(const List<T>& other)
{
   root.previous = root.next = &root;
   ListIterator<Node*> iterator;
   for (iterator = other.Begin(); iterator != other.End(); ++iterator)
   {
      this->Add(*iterator);
   }
   return *this;
}

Das geht wirklich, kaschiert aber wenn ich das richtig sehe nur die fehlerhafte Node-Klasse.

Student83

camper schrieb:

Der Code sagt mir, dass du schon mal was von der Regel der Drei gehört hast, und auch weist, wie man Copy-ctor , copy-Zuweisung und den Destruktor geeignet implementieren kann.

Das stimmt, ich dachte allerdings dass die Compilergenerierten Konstruktoren/Zuweisungsn hier erstmal ausreichen würden.

camper schrieb:

Zu node-copy wurde schon etwas gesagt, allerdings beginnt das Problem viel eher:
template <class T>
class ListNode
{
public:
    ListNode();
    ListNode(const ListNode& listNode);

    ListNode* next;
    ListNode* previous;
    T value;
};

template <class T>
ListNode<T>::ListNode() :
    value()
{
    next = previous = nullptr;
}

template <class T>
ListNode<T>::ListNode(const ListNode& listNode)
{
    previous = listNode.previous;
    next = listNode.next;
    value = listNode.value;
}
Abgesehen von der fehlenden Zuweisung und dem fehlenden Destruktor, die logisch folgen müssten, sind beide Konstruktoren vollkommen unbrauchbar.

Den Destruktor habe ich extra weggelassen, da es in diesem nichts zu tun gibt. Der Copy-Konstruktor ist auch überflüssig, hatte ich nur zu Testzwecken geschrieben. Der Compilergenerierte Code reicht hier aus.

camper schrieb:

1. In jedem Falle muss der Erzeuger des Nodes diese "nachinitialiseren" und die Zeiger richtig setzen. Das bedeutet umgekehrt, dass die Konstruktoren ihrem Zwck nicht gerecht werden.

Ok, in der stl findet man folgendes:

struct _List_node_base {
  _List_node_base* _M_next;
  _List_node_base* _M_prev;
};

template <class _Tp>
struct _List_node : public _List_node_base {
  _Tp _M_data;
};

Ich habe halt anstatt der struct eine class genommen und die Zeiger zusätzlich noch mit nullptr und den value mit dem Default-Konstruktor initialisiert, da ohne diesen das Programm abgestürzt ist.

camper schrieb:

2. Der Copy-Konstruktor erzeugt keine äquivalente Kopie. Das Kopieren der Zeiger aus dem Ursprungsobjekt ist vollkommen sinnlos, da der neue Knoten trotzdem nicht Teil der Liste wird. Das heißt, dass es gar keine äquivalente Kopien geben kann. Also sollte das Kopieren von Nodes überhaupt verboten sein. Und wären Copy-ctor und Copy-Zuweisung verboten, würde der Rest auch nicht kompilieren, insbesondere der Copy-ctor von Node (der schon kritisiert wurde) würde dem Compiler Probleme bereiten.

Klingt prinzipiell gut.

camper schrieb:

Gegenwärtig sind die Zuständigkeiten für die Initilisierung bei dir verteilt. Das ist immer schlecht.
Benutze entweder dumme ListNodes und lasse List die gesamte Arbeit machen, oder mach die ListNodes intelligent genug, so dass sie sich um den Zeigerkram selbst kümmern können.

Wie sieht denn dann ein dummer ListNode genau aus?

Student83

Der Fehler liegt also in den folgenden Zeilen Code. Nur wie bringe ich jetzt den Copy-Konstruktor, den Zuweisungsoperator und die Copy-Funktion miteinander in Einklang? Ich stehe gerade total auf dem Schlauch.

Node::Node(const Node& node)
{
	*this = *node.Copy();
}

Node* Node::Copy() const
{
	Node* node = new Node();
	ListIterator<Node*> iterator;
	for (iterator = nodes.Begin(); iterator != nodes.End(); ++iterator)
	{
		const Node* current = *iterator;
		Node* newNode = new Node(*current);
		node->nodes.Add(newNode);
	}
	node->text = text;
	return node;
}

Node& Node::operator=(const Node& node)
{
	if(this != &node)
	{
        // ?
	}
	return *this;
}

Du hast viele Fehler/Probleme. Der Fehler der dein aktuelles Problem verursacht hat, liegt in List. In dem operator= von node kann nicht viel anderes drinnen stehen als:

Node& Node::operator=(const Node& node)
{
    if(this != &node)
    {
        text = node.text;
        nodes = node.nodes //und hier brauchst du halt wieder operator= von List. Hilft alles nichts. 
    }
    return *this;
}

Der compiler generierte operator= für node ist so schon in ordnung, nur der von List stimmt halt nicht
Und bitte nicht meinen code direkt übernehmen, da muss man vorher eigentlich noch die "alte" Liste, soweit sie existiert löschen.

Die Funktion copy sollte eigentlich schlicht und einfach nicht existieren. Dazu is doch der copy-ctor da!

//"einfache" variante
Node::Node(const Node& node)
{
    *this = node;
}

//oft bevorzugte variante (aus verschiedenen Gründen)
Node::Node(const Node& node) : text(other.text), node(other.nodes)
{
}

Das wars. Die Funktion copy löscht du am besten und vergisst sie. Hast du einen Node und willst davon eine copy, machst du einfach:

Node node1;
Node node2(node1); //voila, node2 ist eine copy von node1

Dazu brauchst du halt noch einen korrekten copy-ctor und operator= von List. Und schon läuft alles wunderbar.

Student83

Ok, so langsam check ichs. Noch eine Frage. Wenn ich folgenden Code nehme,

Node::Node(const Node& node) : text(other.text), node(other.nodes)
{
}

Node& Node::operator=(const Node& node)
{
    if(this != &node)
    {
        text = node.text;
        nodes = node.nodes //und hier brauchst du halt wieder operator= von List. Hilft alles nichts.
    }
    return *this;
}

dann muss ich im Fall dass die Node-Klasse geändert wird den Copy-Kontruktor und den Zuweisungsoperator updaten. Im Thread zum Überladen von Operatoren habe ich folgenden Zuweisungsoperator gefunden:

Node& Node::operator=(const Node& node)
{
	if(this != &node)
	{
		Node tmp(node);
        std::swap(*this, tmp);
	}
	return *this;
}

Kann ich das so machen? Wäre halt weniger arbeit und auch weniger fehleranfällig, da nun nur noch der Copy-Kontruktor im Fall einer Änderung angepasst werden muss.

http://www.cplusplus.com/reference/algorithm/swap/

Swap ruft wieder den copy-ctor und operator= auf. ->Endlosschleife. Das geht so also nicht - es sei denn du schreibst dir ein eigenes swap, dass das vermeidet.
Hat Q btw schon erwähnt

Du kannst prinzipiell den copy-ctor über den operator= implementieren, da musst dus dann auch nur einmal machen - allerdings hast du dann das Problem, das im copy-ctor erst alle member default-konstruiert werden, das machts etwas ineffizient. Mir ist das meistens egal, weil mich die doppelte code-Haltung auch normalerweise nervt.

Node::Node(const Node& node)
{
    *this = node;
}

Node& Node::operator=(const Node& node)
{
    if(this != &node)
    {
        text = node.text;
        nodes = node.nodes;
    }
    return *this;
}

Du musst copy-ctor und Zuweisungsoperator von Node genau dann updaten, wenn du was an den enthaltenen Variablen von Node änderst. Falls du nur Methoden änderst musst du natürlich nichts machen.

Student83

Ja, aber moment mal. Der Code

kleiner Troll schrieb:

Node::Node(const Node& node)
{
    *this = node;
}

Node& Node::operator=(const Node& node)
{
    if(this != &node)
    {
        text = node.text;
        nodes = node.nodes;
    }
    return *this;
}

funktioniert auch nur wenn ich eine Liste von
Objekten habe. Z.B.

List<Node> myList;

Ich habe aber eine Liste von Zeigern

List<Node*> myList;

Somit würde also doch nur eine flache Kopie erstellt.
Darum habe ich aus den Zeigern in der Liste mal Objekte gemacht. Allerdings bekomme ich nun den Compilerfehler das Node undefiniert ist. Kann eine Klasse überhaupt ein Objekt von sich selber enthalten?

EDIT:
Ich werde es jetzt so machen, dass im Copy-Konstruktor und Zuweisungsoperator nur flache Kopien erzeugt werden und dann wie ursprünglich angedacht eine Copy-Methode anbieten die eine tiefe Kopie zurückgibt. Alles andere wäre auch eine irrsinnige Rumkopiererei.

Vergiss deine Copy-Methode. Wirklich, vergiss sie einfach mal.

Ja, aber moment mal. Der Code
[...]
funktioniert auch nur wenn ich eine Liste von
Objekten habe. Z.B.
[...]

Ich habe aber eine Liste von Zeigern
[...]

Somit würde also doch nur eine flache Kopie erstellt.

Das ist korrekt. Allerdings hat Node die Node* in nodes nicht selbst erstellt, damit auch keinen "Besitz" darauf. Und hier krankt es auch wieder fundamental an deiner Idee der Copy-Methode. Beim ersten Aufruf in deinem code werden die Node* extern erstellt, und Node nur übergeben. Der Aufruf einer Copy-Methode zur tiefen copy stellt dich vor das dilemma: wer ist verantwortlich für die Node* in Node::nodes? Ist Node selbst dafür verantwortlich, löscht es die Node* bei einer normalen konstruktion, obwohl die eigentlich dem ersteller von Node gehören. Löscht es hingegen nicht, hast du bei einer Konstruktion über ein copy keinen mehr, der sich für die Zeiger verantwortlich fühlt -> memory leak.

Dein ganzer Ansatz ist einfach fundamental kaputt. Wenn du mir sagst, was Node überhaupt darstellen soll (der name ist extrem unaussagekräftig) kann ich dir vielleicht sagen wie man das halbwegs erträglich designt.

Student83

kleiner Troll schrieb:

Das ist korrekt. Allerdings hat Node die Node* in nodes nicht selbst erstellt, damit auch keinen "Besitz" darauf. Und hier krankt es auch wieder fundamental an deiner Idee der Copy-Methode. Beim ersten Aufruf in deinem code werden die Node* extern erstellt, und Node nur übergeben. Der Aufruf einer Copy-Methode zur tiefen copy stellt dich vor das dilemma: wer ist verantwortlich für die Node* in Node::nodes? Ist Node selbst dafür verantwortlich, löscht es die Node* bei einer normalen konstruktion, obwohl die eigentlich dem ersteller von Node gehören. Löscht es hingegen nicht, hast du bei einer Konstruktion über ein copy keinen mehr, der sich für die Zeiger verantwortlich fühlt -> memory leak.

Dein ganzer Ansatz ist einfach fundamental kaputt. Wenn du mir sagst, was Node überhaupt darstellen soll (der name ist extrem unaussagekräftig) kann ich dir vielleicht sagen wie man das halbwegs erträglich designt.

Da hätte ich vielleicht wirklich mehr dazuschreiben sollen. Das ganze ist teil eines XML-Parsers. Der Übersichtlichkeit halber habe ich nur den ganzen kram zum Einlesen der Files weggelassen. Die Nodes werden auch nicht extern erstellt. Das habe ich hier nur desshalb gemacht, um einen Beispielbaum aus Nodes zu erzeugen den ich zur Demonstration des Fehlers ausgeben kann.

Dann muss der copy-ctor eine tiefe copy anfertigen. Alles andere is ein logischer Fehler im copy-ctor.

Oder anders gesagt:
Machst du keine tiefe copy, crasht das programm hier, sobald der copy-ctor mal verwendet wurde:

Node::~Node()
{
    ListIterator<Node*> iterator;
    for (iterator = nodes.Begin(); iterator != nodes.End(); ++iterator)
    {
        delete *iterator;
    }
}

{
Node node1;
Node node2(node1);
} //hier werden die destrukturen von node1 und node2 aufgerufen:

Node::~Node() //node1, hier klappt noch alles
{
    ListIterator<Node*> iterator;
    for (iterator = nodes.Begin(); iterator != nodes.End(); ++iterator)
    {
        delete *iterator;
    }
}

Node::~Node() //node2
{
    ListIterator<Node*> iterator;
    for (iterator = nodes.Begin(); iterator != nodes.End(); ++iterator)
    {
        delete *iterator; //der bereich auf den *iterator zeigt wurde von node1 schon gelöscht! crash!
    }
}

Dann darfst du beim operator= nicht vergessen, vorher alten speicher freizugeben, bevor du zuweist, sonst hast du wieder ein memory leak.

Im Grunde musst du, sobald du irgendwo in ner Klasse ein new (ohne delete in derselben funktion) hast, immer einen destruktor schreiben der das freigibt, einen operator= der es erst freigibt, dann den speicher neu reserviert und eine Kopie der daten im anderen Objekt erstellt und einen copy-ctor der eine ebenfalls mit new eine Kopie der Daten die mit new angelegt wurden des anderen Objekts macht. Dann ist aber auch eine copy-funktion so wie du sie vorhattest automatisch sinnlos. Nur als private funktion um gemeinsamen code aus copy-ctor und operator= macht das vielleicht Sinn.