Array einer eigenen Klasse dynamisch erweitern ->Frage zum Destruktor [GELÖST]

butterbemme

Hiho,

ich hab folgende Klasse:

class Personenliste{
	Person* p;
	size_t gr;
	int anzahl;
public:
	Personenliste(size_t s=1);
	~Personenliste();

	int getGröße();
	int getAnzahl();
	Person getPerson(int index);
	void addPerson(Person per);
};

Und die addPerson Funktion schaut so aus:

void Personenliste::addPerson(Person per){
	p[anzahl]=per;
	anzahl++;
	if(anzahl==gr){
		Person* neu=new Person[gr=gr+1];
		~Personenliste();
		p=neu;

	}
	cout << "Person eingefuegt" << endl;
}

Diese Funktion soll immer wenn das Feld von Personen p voll ist, neuen Speicherplatz mit der bisherigen Feldgröße +1 bereitstellen und den alten nicht mehr gebrauchten freigeben.
Ist mein Ansatz hier richtig, benutze ich den Destruktor hier richtig?

Hier noch Kon- und Destruktor

Personenliste::Personenliste(size_t s){
	p=new Person[gr=s];
	anzahl=0;
}

Personenliste::~Personenliste(){
	delete[] p;
}

edit: Mir fällt grad auf, dass der Aufruf des Destruktors

~Personenliste()

in der Funktion addFunktion mir eine Fehlermeldung auswirft:

error C2675: Unärer Operator '~': 'Personenliste' definiert diesen Operator oder eine Konvertierung in einen für den vordefinierten Operator geeigneten Typ nicht

Nur mit "delete[] p" funktionierts, was ja eigentlich der einzige Inhalt von
"~Personenliste()" ist.
Woran liegt das?

Belli

butterbemme schrieb:

Ist mein Ansatz hier richtig, benutze ich den Destruktor hier richtig?

Nein. Es gibt nur gaaaaaaaaaanz wenige Fälle, wo es Sinn macht, den Destruktor ausdrücklich aufzurufen, so wenig, dass ich Dir keinen nennen kann. Merk Dir einfach: Das tut man nicht!
Gib den Speicher mit delete[] frei.

Noch besser, benutze statt eines Arrays std::vector, der ist genau dafür gemacht!

butterbemme schrieb:

Ist mein Ansatz hier richtig, benutze ich den Destruktor hier richtig?

Nein. Destruktor hat da NICHTS zu suchen. Du willst da nur ein neues Array anlegen, das alte freigeben und den Member-Pointer umsetzen. Destruktor ist was anderes, das signalisiert "Objekt wird zerstört", und das willst du ja nicht, oder?

Ansonsten: Ist das eine Übung, oder kennst du einfach std::vector noch nicht?

Travor

Normalerweise wird ein Objekt automatisch gelöscht, es sei denn es liegt auf dem Heap. Nur die Objekte, die mit new/~~malloc~~ erzeugt wurden landen dort.
Deswegen wird ein Destruktor nur bei Heap-Objekten explizit aufgerufen. Und zwar mit dem Schlüsslwort «delete».

Object* heapO = new Object;
delete heapO;

das bedeutet anstatt

~Personenliste();

muss Du

delete this

verwenden.

Das ist zu 99% der Fälle eine sehr schlechte Entscheidung. In dem Moment ist deine Instanz quasi sofort weg. Die gibt es nicht mehr!!
Um «delete this» in eine Membermethode schreiben zu können muss Du viele Dinge sicherstellen:
1. Die Instanz wurde mit new erzeugt.
2. Die Methode greift nicht mehr auf die Member von der Instanz und ist die letzte für diese Instanz aufgerufene Methode.
2. Es wird nie mehr auf diese Instanz zugegriffen. ( keine Querverweise ).

Dein Problem kannst du einfach lösen indem du anstatt eines Arrays einen von den STL-containern nimmst: z.B.

std::vector< Person > v;
p.push_back( p )

// oder

std::deque< Person > d;
d.push_back( p );

// oder std::list< Person >, std::queue.. usw.

Hier ist ein Beispiel:

#include <vector>

class Personenliste
{
public:
	Personenliste( size_t anzahlElemente = 1 ) : leute() { leute.reserve( anzahlElemente ); }
    //~Personenliste();

    size_t Größe() const
	{
		return leute.capacity();
	}

    size_t Anzahl() const
	{
		return leute.size();
	}

    Person GetPerson( size_t index ) const		// checked.
	{
		return leute.at( index );
	}

	Person operator []( size_t index ) const	// unchecked
	{
		return leute[ index ];
	}

	void addPerson( Person& const mensch )
	{
		leute.push_back( mensch );
	}

private:
    std::vector< Person > leute; // :)
};

krümelkacker

butterbemme schrieb:

Und die addPerson Funktion schaut so aus:

void Personenliste::addPerson(Person per){
	p[anzahl]=per;
	anzahl++;
	if(anzahl==gr){
		Person* neu=new Person[gr=gr+1];
		~Personenliste();
		p=neu;
		
	}
	cout << "Person eingefuegt" << endl;
}

Auweia! per bitte per ref-auf-const übergeben. Die Puffergröße sollte sich verdoppeln und nicht immer nur ein Element größer werden. Damit erreichst Du dann beim wiederholten addPerson amortisiert konstante Laufzeit pro Aufruf statt eine Laufzeit, die abhängig von der Größe des Puffers ist. Destruktor ruft man (so gut wie) nie selbst auf! Jedenfalls nicht hier! Die alten Personen werden nirgenswo in den neuen Puffer kopiert. Du verletzt die Dreierregel und bekommst murksiges Verhalten, wenn Du so ein Personenliste Objekt kopierst.

class Personenliste kannst du komplett gegen std::vector<Person> eintauschen. std::vector hat all diese genannten Probleme nicht.

butterbemme

Also std::vector is mir bekannt.
Ich versuch mich grade in selbstdefinierte Klassen einzuarbeiten und will ein bisschen mit new und delete rumspielen, deswegen mein etwas umständlicher Code.
Ist also sozusagen eine Übung.

@Belli: du meinst ich solle nur "delete[] p" benutzen. Aber genau das und zwar nur das, steht ja auch in meinem selbstdefinierten Destruktor ~Personenliste().
Wo ist dann also der Unterschied? Oder wird da implizit noch etwas anderes im Hintergrund gemacht.

krümelkacker schrieb:

... Die alten Personen werden nirgenswo in den neuen Puffer kopiert. Du verletzt die Dreierregel und bekommst murksiges Verhalten, wenn Du so ein Personenliste Objekt kopierst...

Jo da hast du Recht. Um die Dreierregel hab ich mich noch nicht gekümmert, und auch das Kopieren der Personen aus dem alten Speicherbereich fehlt noch.

Dann lass ich wohl vorerst die Finger von selbstdefinierten Destruktor.
Aber zu guter Letzt würd mich noch interessieren warum der direkte Destruktoraufruf (~Personenliste()) in der Funktion "addPerson()" nicht funktioniert hat.
Ich bekomm da immer

Unärer Operator '~': 'Personenliste' definiert diesen Operator oder eine Konvertierung in einen für den vordefinierten Operator geeigneten Typ nicht

Muss ich diesen Operator erst für mein Vorhaben extra definieren. Dachte eigentlich Destruktoren haben diese Form und werden dann auch so aufgerufen

Travor

void Personenliste::addPerson( Person& const person )
{
    if ( anzahl >= gr )
    {
        Person* tmp = 0;
        size_t neuAnzahl = gr + gr/ 2;
        if ( neuAnzahl < gr + 1  &&  gr + 1 > gr )
        {
            neuAnzahl = gr + 1;
        }
        else throw std::bad_alloc( "Speicher voll" ); //  :) 

        Person* tmp = new ( nothrow ) Person[ neuAnzahl ];
        if ( tmp == 0 ) throw std::bad_alloc();

        memcpy( tmp, p, min( gr, neuAnzahl ) * sizeof ( Person ) );
        delete[] p;
        p = tmp;
        gr = newAnzahl;
    }
    p[ anzahl ] = person;
    ++anzahl;
}

vorausgesetzt Person ist POD..

DocShoe

Nimm statt memcpy lieber std::copy , das funktioniert auch mit non-POD Typen und ist etwas handlicher imho.

void Personenliste::add_person( const Person& p )
{
   if( Anzahl >= gr )
   {  
      size_t NewSize = gr * 2;
      Person* buffer = new Person*[NewSize];

      std::copy( p, p + gr, buffer );

      delete[] p;
      p = buffer;
      gr = NewSize;
   }
   p[Anzahl] = person;
   ++anzahl;   
}

Belli

butterbemme schrieb:

[...]
@Belli: du meinst ich solle nur "delete[] p" benutzen. Aber genau das und zwar nur das, steht ja auch in meinem selbstdefinierten Destruktor ~Personenliste().
Wo ist dann also der Unterschied? Oder wird da implizit noch etwas anderes im Hintergrund gemacht.

[...]

Dachte eigentlich Destruktoren haben diese Form und werden dann auch so aufgerufen

Ja, da wird - wie hier auch schon jemand schrieb - Dein Objekt zerstört.
Destruktoren haben diese Form, aber wie auch schon jemand schrieb, sie werden nicht explizit aufgerufen.

{
   objekt a;
}// hier wird wegen des Verlassens des Scopes der Destruktor von a implizit aufgerufen.
objekt *b = new b(); // hier muss es natürlich: objekt *b = new objekt(); heißen,  :) 
delete b; // hier wird von delete der Destruktor von b implizit aufgerufen

Anders solltest Du bis auf Weiteres Destruktoren nicht aufrufen.

butterbemme

Okay, ich denke ich habs soweit verstanden.
Danke für eure Hilfe.

realKoala

hey arbeite dich erstmal durch ein klassen tut... so kommste nicht weit ^^

hier noch ne regel falls du weiter auf dem heap arbeiten möchtest

definierst du einen dieser drei selbst:
destructor
copy constructor
zuweisungs operator

musst du in der regel alle selbst schreiben, aber naja kommt noch ;P

314159265358979

Üblicherweise ruft man einen Dtor nur dann explizit auf, wenn das Element mit einer Form von placement-new erzeugt wurde.

TyRoXx

DocShoe schrieb:

Nimm statt memcpy lieber std::copy , das funktioniert auch mit non-POD Typen und ist etwas handlicher imho.

void Personenliste::add_person( const Person& p )
{
   if( Anzahl >= gr )
   {  
      size_t NewSize = gr * 2;
      Person* buffer = new Person*[NewSize];

      std::copy( p, p + gr, buffer );

      delete[] p;
      p = buffer;
      gr = NewSize;
   }
   p[Anzahl] = person;
   ++anzahl;   
}

Was ist mit gr == 0 ?
Warum wird new[] mit dem falschen Typ benutzt?
Was ist, wenn copy wirft?
Was sind anzahl und Anzahl ?
Warum nicht gleich vernünftig mit Allocator?

butterbemme

Ich bin mal so frei und werde den Thread als "gelöst" markieren. Wenns noch Unstimmigkeiten unter den einzelnen Postern gibt, können die den Thread ja trotzdem weiterfürhen

Array einer eigenen Klasse dynamisch erweitern -&gt;Frage zum Destruktor [GELÖST]

Array einer eigenen Klasse dynamisch erweitern ->Frage zum Destruktor [GELÖST]