Kopierkonstruktor passt das so?

[Stromberg]

Also ich mach das schon richtig oder? Bei nem Kopierkonstruktor muss ich doch nur alles Zeiger neu definieren, weil sonst beide Objekte auf die gleiche Speicherstelle auf dem Heap verweisen würden? Nicht aber normale Variablen ? Die muss man doch nicht irgendwie neu definieren oder so? Aber warum muss ich die nicht neu definieren? Macht das der Compiler automatisch oder wie läuft das ab? Hier mal mein Beispiel:

#include <iostream>
#include <string>
using namespace std;

class Cat
{
    public:
    Cat(int Age,int Weight,string Name);
    Cat(const Cat &rhs);
    ~Cat();
    int GetAge() const {return *itsAge;}
    int GetWeight() const {return *itsWeight;}

    private:
    int *itsAge;
    int *itsWeight;
    string itsName;
};

Cat::Cat(int Age,int Weight,string Name)
:itsAge(new int),itsWeight(new int),itsName(Name)
{
    *itsAge=Age;
    *itsWeight=Weight;
}

Cat::Cat(const Cat &rhs)
:itsAge(new int),itsWeight(new int)
{
    *itsAge=rhs.GetAge();
    *itsWeight=rhs.GetWeight();
}

Cat::~Cat()
{
    delete itsAge;
    delete itsWeight;
}

int main()
{

    return 0;
}

Passt das so? Dankeschön schon mal im Voraus.

[akari]

Hallo

Das du den Wert in den Pointern kopierst und nicht die Adresse selber ist schon richtig.

bis bald
akari

[CStoll]

Wenn du keinen Copy-Ctor hast, erzeugt der Compiler dir einen - und der ruft die Copy-Ctoren der Member auf (Problem bereiten hier nur Member, die keinen in deinem Sinn vernünftigen Copy-Ctor haben - z.B. Zeiger). Wenn du deinen eigenen Copy-Ctor definierst, mußt du dich selber um die Kopier-Operationen kümmern (wenn nichts anderes in der Initialisierungsliste angegeben ist, verwendest du den Default-Ctor für die Member) - auch bei "normalen" Elementen:

Cat::Cat(const Cat &rhs)
:itsAge(new int(rhs.GetAge())),itsWeight(new int(rhs.GetWeight())),itsName(rhs.GetName())
{}

PS: Warum legst du die beiden int-Werte überhaupt auf dem Heap an? Die sind doch klein genug, um direkt im Objekt untergebracht zu werden.

[Stromberg]

@CStoll
Is nur eine Übung. Darum hab ich die auf dem Heap angelegt :). Übung macht den Meister^^(also hoff ich mal)
EDIT:
Ah ich sehs grad man kann anscheinend auch alles in die Initialisierungsliste stecken! Ist das besser so? Wird dadurch das Programm schneller? Was istn eure Meinung dazu, wie mach ich das am besten in Zukunft?

[CStoll]

Ja, das Programm wird dadurch etwas schneller, weil du dir eine Zuweisung einsparen kannst:

class test
{
  string data;
public:
  //Default-Ctor und anschließende Zuweisung
  test(const string& val) {data=val;}
  //Copy-Ctor
  test(const string& val):data(val) {}
};

(außerdem gibt es bestimmte Fälle (Referenzen, const Member, Member ohne Default-Ctor,...), bei denen du die Werte NUR über die Initialisierungsliste vorbelegen kannst)

[viande]

Ich will ja jetzt hier nicht klugscheißen, aber wenn du dir ganz sicher sein willst, dass keine Speicherlücken entstehen oder deine Objekte in einem inkonsistenten Zusatnd sind, solltest du freien Speicher (auf dem Heap) folgendermaßen Nutzen (das gilt nur, wenn die Objekte, die abgelegt werden auch nur dem objekt alleine gehören, in dem sie gespeichert werden):

Cat::Cat(const Cat &rhs)
: itsName(rhs.itsName)
{
    int *t_age = 0, *t_weight = 0;
    try
    {
        t_age = new int(*rhs.itsAge);
        t_weight = new int(*rhs.itsWeight);
    }
    catch(...)
    {
        delete t_age;
        delete t_weight;
        throw;
    }
    itsAge = t_age;
    itsWeight = t_weight;
}

Dadurch wird sichergestellt, dass, falls keine Speicher angefordert werden kann, dass dein Objekt so bleibt, wie es war. Falls außerdem new erst bei t_weight fehlschlägt, wird t_age wieder freigegeben. Das delete auf t_weight wird nicht fehlschlagen, weil new im Falle eines Fehlers eine Ausnahme wirft und der Pointer damit auf 0 bleibt, welcher vom delete nicht weiter angefasst wird.
Nebenbei solltest du auch daran denken, den Zuweisungsoperator zu überladen. Sonst wird das zu Problemen führen:

Cat kitty(2, 10, "Kitty");
Cat kitty2;
kitty2 = kitty; //hier werden die Zeiger kopiert.

kitty.~Cat(); //hier wird der Speicher freigegeben
kitty2.~Cat(); //und hier versucht das Objekt dann Speicher freizugeben, der ihm nicht gehört.

[CStoll]

viande schrieb:

Ich will ja jetzt hier nicht klugscheißen, aber wenn du dir ganz sicher sein willst, dass keine Speicherlücken entstehen oder deine Objekte in einem inkonsistenten Zusatnd sind, solltest du freien Speicher (auf dem Heap) folgendermaßen Nutzen (das gilt nur, wenn die Objekte, die abgelegt werden auch nur dem objekt alleine gehören, in dem sie gespeichert werden):

Cat::Cat(const Cat &rhs)
: itsName(rhs.itsName)
{...}

Und was hast du da bei einem Fehler? Eine benannte Katze ohne Alter und Gewicht. Wenn bei der Initialisierung etwas schiefgegangen ist, sollte auf keinen Fall ein halbfertiges Objekt zurückbleiben (das ist mitunter noch schlimmer als ein Speicherleck).

[Braunstein]

Entschuldigung, aber deine Version des CopyCTors ist ziemlicher Unsinn. Wenn da eine Exception auftritt machst du zwar ein delete weist aber danach die ungültigen Zeiger zu. Was willst du mit dem ungültigen Objekt dann überhaupt machen?
Wenn in einem Konstruktor oder CopyKonstruktor eine Exception fliegt sollte die nach draußen gehen, und dem Programm sagen, dass dein Objekt nicht erstellte werden konnte.

[viande]

Braunstein schrieb:

Entschuldigung, aber deine Version des CopyCTors ist ziemlicher Unsinn. Wenn da eine Exception auftritt machst du zwar ein delete weist aber danach die ungültigen Zeiger zu. Was willst du mit dem ungültigen Objekt dann überhaupt machen?
Wenn in einem Konstruktor oder CopyKonstruktor eine Exception fliegt sollte die nach draußen gehen, und dem Programm sagen, dass dein Objekt nicht erstellte werden konnte.

Ups, hatte das erneute Werfen der Ausnahme vergessen :o

Und was hast du da bei einem Fehler? Eine benannte Katze ohne Alter und Gewicht. Wenn bei der Initialisierung etwas schiefgegangen ist, sollte auf keinen Fall ein halbfertiges Objekt zurückbleiben (das ist mitunter noch schlimmer als ein Speicherleck).

Das mit der Konsistenz war hier vielleicht fehl am Platz. Das ist bei ähnlicher Nutzung eher auf den Zuweisungsoperator anzuwenden.
Aber hier auch das gleiche: ich hatte das throw vergessen. Wenn nun keiner die Exception abfängt, kann ich das auch nicht ändern. Wäre aber bei der Version oben auch nicht anders: entweder man hat ein Speicherleck oder zwei 0-Zeiger. Wenn man diese nicht weiter überprüft geht man damit auch baden. So kann man aber die Ausnahmen abfangen und das Programm suber beenden.

[Braunstein]

Wenn die Exception vom Programm nicht gefangen wird, beendet sich das Programm sowieso und dann spielen Speicherlecks auch keine Rolle mehr.

[viande]

Mh, sagen wir mal, ich lege da zwei Objekte an die, die jeweils 1 MB groß sind. Außerdem unterstützt mein Betriebssystem kein Garbage Collection.
Nun kriege ich für das 2. Objekt keinen Speicher und das erste liegt als Leiche im Speicher. 1MB verschwendet ... Weit hergeholt, weiß ich selber, ich bin lieber auf alle Eventualitäten vorbereitet, als das am Ende doch mal was schief geht.
Bezogen auf den op=: wenn nun jemand die Ausnahme abfängt und dafür sorgt, dass das Programm ansonsten normal weiterläuft, habe ich als Ersteller dieser Klasse dafür Sorge zu tragen, dass ein Objekt nach so einem Fehlschlag konsistent bleibt. Bei dem Kopierkonstruktor sieht das natürlich anders aus, weil es ja vorher gar kein Objekt gab.

[Braunstein]

Nun, die Betriebssysteme, die ich kenne räumen den Speicher nach beendigung eines programmes schon auf.
Beim op= ist das natürlich anders. Hier kann man aber schön das swap-Idiom von Herb sutter verwenden und hat dann auch keine Probleme mehr.

[gesunder Menschenverstand]

struct Foo {
     Foo() : a(0), b(0) {
          Megabyte* a = new Megabyte;
          Megabyte* b = new Megabyte;
     }
     ~Foo() {
          delete a; delete b;
     }
     Megabyte* a,b;
};

Das geht doch völlig in Ordnung. Wenn ein new schiefgeht können 2 Dinge passieren:
- Die Exception wird nicht gefangen, dann ist eh alles egal - Ende Gelände.
- Die Exception wird gefangen, dann werden auch die Objekte vom Stack genommen, der Destruktor übernimmt das Löschen der bereits angelegten Objekte.

[THX 1138]

und wenn du beim op= von Megabyte ne exception hast biste verratzt

[gesunder Menschenverstand]

Ich sehe da kein Problem...

//in Foo
Foo& operator=(Foo const& rhs) {
    delete a; delete b;
    a = b = 0;
    a = new Megabyte(*rhs.a);
    b = new Megabyte(*rhs.b);
}

hier speziell braucht man noch ein this != &rhs

[gesunder Menschenverstand]

Oh du hast von Megabyte geschrieben das hab ich gar net gesehn. Was hat der mit der ganzen Sache zu tun, den verwend ich nie...

[Braunstein]

Genau hier gibts ein Problem. Wenn das erste new im op= eine Exception wirft hast du ein uninitialisiertes Objekt. Mach es so

struct Foo {
     Foo() : a(new Megabyte), b(new Megabyte) {}
     ~Foo() {
          delete a; delete b;
     }
     void Swap(Foo& src)
     {
        std::swap(a, src.a);
        std::swap(b, src.b);
     }
    Foo& operator=(Foo const& rhs) 
    {
       Foo tmp(rhs);
       Swap(tmp);
       return *this;
    }
     Megabyte* a,b;
};

[THX 1138]

Oder nimm einfach shared_ptr aus boost oder tr1

[camper]

gesunder Menschenverstand schrieb:

- Die Exception wird gefangen, dann werden auch die Objekte vom Stack genommen, der Destruktor übernimmt das Löschen der bereits angelegten Objekte.

Keineswegs. Der Destruktor wird nur für lebende Objekte aufgerufen, das heißt, der Konstruktor muss normal beendet worden sein. Das Werfen einer Exception innerhalb des Konstruktors (und sei es in der Initialisierungsliste) verhindert das. Grundsätzlich ist das Problem ohne RAII für mehr als eine Resource, deren Anforderung scheitern kann, kaum zu bändigen. Möglich ist es, aber nicht empfelenswert:

struct Foo
{
    Foo()
    try
    : a( ( a = NULL, new Megabyte() ) ),
      b( new Megabyte() )
    {}
    catch (...)
    {
        delete a;
    }
    Foo(const Foo& other)
    try
    : a( ( a = NULL, new Megabyte( *other.a ) ) ),
      b( new Megabyte( *other.b ) )
    {}
    catch (...)
    {
        delete a;
    }
     ~Foo()
     {
          delete b;
          delete a;
     }
    Foo& operator=(Foo const& rhs)
    {
        Megabyte* new_a = new Megabyte( *rhs.a );
        try
        {
            Megabyte* new_b = new Megabyte( *rhs.b );
            delete b;
            delete a;
            a = new_a;
            b = new_b;
            return *this;
        }
        catch (...)
        {
            delete new_a;
            throw;
        }
    }
    Megabyte* a;
    Megabyte* b;
};

Bei Verwendung von scoped_ptr, auto_ptr oder ähnlichem wird das Ganze dagegen fast trivial.

[THX 1138]

Warum löscht du im try Block nur a?
Soweit ich weis ist es save auch nen null pointer zu löschen, ein delete b würde also nichts schaden..

[Braunstein]

Bei seiner Variante ist b kein Nullpointer. Warum sollte er b auch löschen? Wenn eine Exception fliegt ist b sowieso nicht initialisiert und wenn keine fliegt muß man auch nichts löschen.