Einfachverkettete Liste so in Ordnung?



  • Hallo an die Forumsmitglieder,

    ich habe eine einfachverkettete Liste geschrieben. Das Programm funktioniert einwandfrei. Ich würde mich nur gern nochmal erkundigen, ob ich alles richtig gemacht habe bzw. ob man es anders machen sollte. Es geht mir nur um das Prinzip. Ich habe ein einfaches Programm geschrieben, mit einer Klasse, in der nur ein Konstruktor, Destruktor, eine Einfügefunktion und ein Outputstream enthalten sind. In der Klasse existieren 2 Zeiger. Einer zeigt auf das aktuelle Element, das andere auf das Folgende. So zumindest habe ich das Prinzip mit der einfachverketteten Liste verstanden.

    Wäre toll, wenn ihr mir ein Feedback geben könntet zu dem Programm:

    [code="cpp"]
    #include <iostream>

    using namespace std;

    class Einfachliste{

    private:
    string vname;
    string nname;

    Einfachliste* beginn;
    Einfachliste* naechste;

    public:
    void neuername(const char*, const char*);
    Einfachliste();
    ~Einfachliste();
    friend ostream& operator<<(ostream& os, const Einfachliste&);

    };

    Einfachliste::Einfachliste(){

    this->beginn = NULL;
    this->naechste = NULL;
    this->vname = "Eintrag";
    this->nname = "Eintrag";

    }

    Einfachliste::~Einfachliste(){}

    void Einfachliste::neuername(const char* vname, const char* nname){

    Einfachliste* eintrag;
    eintrag = new Einfachliste;

    eintrag->vname = vname;
    eintrag->nname = nname;

    eintrag->naechste = beginn;
    this->beginn = eintrag;

    }

    ostream& operator<<(ostream& os, const Einfachliste& person)
    {

    Einfachliste* out;
    out = new Einfachliste;
    out = person.beginn;

    while(out!=0)
    {
    os<<"VName: "<<out->vname<<", NName: "<<out->nname<<endl;
    out = out->naechste;
    }

    return os;

    }

    int main(){

    Einfachliste personenregister;

    const char* vnameper = "Albert";
    const char* nnameper = "Schroeder";

    for(int i = 0; i<5; i++){
    personenregister.neuername(vnameper, nnameper);
    }

    cout<<"Die Personenliste"<<endl;
    cout<<personenregister<<endl;

    return 0;
    }

    [/cpp]



  • Sorry, hier der Code

    #include <iostream>
    
    using namespace std;
    
    class Einfachliste{
    
    	private:
    	string vname;
    	string nname;
    
    	Einfachliste* beginn;
    	Einfachliste* naechste;	
    
    	public:
    	void neuername(const char*, const char*);
    	Einfachliste();
    	~Einfachliste();
    	friend ostream& operator<<(ostream& os, const Einfachliste&);
    
    };
    
    Einfachliste::Einfachliste(){
    
    	this->beginn = NULL;
    	this->naechste = NULL;
    	this->vname = "Eintrag";
    	this->nname = "Eintrag";
    
    }
    
    Einfachliste::~Einfachliste(){}
    
    void Einfachliste::neuername(const char* vname, const char* nname){
    
    	Einfachliste* eintrag;
    	eintrag = new Einfachliste;
    
    	eintrag->vname = vname;
    	eintrag->nname = nname;
    
    	eintrag->naechste = beginn;
    	this->beginn = eintrag;
    
    }
    
    ostream& operator<<(ostream& os, const Einfachliste& person)
    {
    
    	Einfachliste* out;
    	out = new Einfachliste;
    	out = person.beginn;
    
    	while(out!=0)
    	{
    		os<<"VName: "<<out->vname<<", NName: "<<out->nname<<endl;
    		out = out->naechste;
    	}
    
    	return os;
    
    }
    
    int main(){
    
    	Einfachliste personenregister;
    
    	const char* vnameper = "Albert";
    	const char* nnameper = "Schroeder";
    
    	for(int i = 0; i<5; i++){	
    		personenregister.neuername(vnameper, nnameper);	
    	}
    
    	cout<<"Die Personenliste"<<endl;
    	cout<<personenregister<<endl;
    
    	return 0;
    }
    


  • Die Dinge die ich beim kurz drüberlesen gesehen habe:

    Zeile 52:

    51  Einfachliste* out;
    52  out = new Einfachliste;
    53  out = person.beginn;
    

    Und generell hast du vergessen, den ganzen mit new allozierten Speicher wieder frei zu geben.

    Sieht ansonnsten OK aus.

    Lg



  • Hallo Singender Holzkübel,

    danke für deine schnelle Antwort. Das Programm funktioniert auch richtig. Nur habe ich diese Frage gestellt, weil in den Beispielen, die ich mir angesehen habe, stets 2 Klassen angelegt worden sind. Einmal für die Daten und einmal für die Kette. Muss man das denn so machen? Oder ist meine Version schon so in Ordnung? Schließlich sind die Zeiger in der Klasse schon enthalten, die stets auf die Einträge zeigen sollen, also das erste und das dem folgende Element.



  • Memory Leak. Gewaltiges.



  • Hallo Nathan,

    wo genau?

    Ich habe in dem Destruktor zusätzlich eingebaut:

    Einfachliste::~Einfachliste(){
    
    	Einfachliste* neu;
    	neu=beginn;
    
    	while(neu!=0){
    
    		if (neu!=0)
    		{
    			delete neu->naechste;
    		}
    		else
    			delete beginn;
    		neu=neu->naechste;
    	}
    
    }
    

    Löst dies das Problem? Den output-Operator habe ich schon angepasst.



  • Sarah98 schrieb:

    Hallo Singender Holzkübel,

    danke für deine schnelle Antwort. Das Programm funktioniert auch richtig. Nur habe ich diese Frage gestellt, weil in den Beispielen, die ich mir angesehen habe, stets 2 Klassen angelegt worden sind. Einmal für die Daten und einmal für die Kette. Muss man das denn so machen? Oder ist meine Version schon so in Ordnung? Schließlich sind die Zeiger in der Klasse schon enthalten, die stets auf die Einträge zeigen sollen, also das erste und das dem folgende Element.

    Eine Kette-Klasse, die die ganzen Daten-Knoten enthält, ist durchaus sinvoll.
    Damit hast du dann z.B. gute Kontrolle über die gesamten Daten.



  • Hallo Singender Holzkübel,

    ok, habe ich gemacht. Aber ist denn mein Memory Leak bzw. der mit new allozierten Speicher mit Hilfe des Dekonstruktors behoben?



  • Sarah98 schrieb:

    Hallo Singender Holzkübel,

    ok, habe ich gemacht. Aber ist denn mein Memory Leak bzw. der mit new allozierten Speicher mit Hilfe des Dekonstruktors behoben?

    Nein, weil dein Destructor-Code nicht so funktioniert, wie du dir das vorstellst. 😛

    Hast du vielleicht aktuelleren Code, mit der Ketten-Klasse? Dann kann ich dir das ganze zeigen.

    Lg



  • Verbesserungsvorschläge:

    - Ändere Deine Liste so, ,dass Du zwischen der Liste und einem Eintrag in der Liste unterscheidest
    - Nutze Initialisierungslisten http://de.wikipedia.org/wiki/Initialisierungsliste
    - beachte die Regel der DREI http://de.wikipedia.org/wiki/Dreierregel
    - include <string> hinzufügen
    - kein using namespace std; im Header

    so die Richtung:

    #include <iostream>
    #include <string>
    
    class Einfachliste {
    public:
        struct Eintrag {
            Eintrag( const std::string& vn, const std::string& nn );
            Eintrag* naechste;
            std::string vname;
            std::string nname; 
        };
    
    private:
        Eintrag* beginn;
    
        // Bem.: macht man Copy-Konstruktor und Assignment-Opeator private, so muss man sie nicht implementieren
        Einfachliste( const Einfachliste& b );  // Regel der DREI beachten
        Einfachliste& operator=( const Einfachliste& b );
    
    public:
        void neuername(const Eintrag& eintrag);
        Einfachliste();
        ~Einfachliste();
        friend std::ostream& operator<<( std::ostream& os, const Einfachliste&); 
    };
    
    int main(){
        using namespace std;
        Einfachliste personenregister;
    
        std::string vnameper = "Albert0";
        const char* nnameper = "Schroeder";
    
        for(int i = 0; i<5; i++, ++vnameper[6] ){  
            personenregister.neuername( Einfachliste::Eintrag( vnameper, nnameper ) );
        }
    
        cout<<"Die Personenliste"<<endl;
        cout<<personenregister<<endl;
    
        return 0;
    }
    

    @Edit Zeile 9: @DocShoe done



  • Korrekturvorschlag:
    Zeile 9 Einfachliste* naechste durch Eintrag* naechster ersetzen.


Log in to reply