3. [Gelöst] Verstehe diesen Copy Assignment Operator nicht. (simple vector)

[Gelöst] Verstehe diesen Copy Assignment Operator nicht. (simple vector)


  • Hallo zusammen,

    ich les grad in einem Buch etwas über vector und da wird auch ein "eigener" vector implementiert. Jetzt gibt es folgenden copy-assignment-operator:

    class Vector {
//...
private:
    void copy(const Vector&);
    int sz;
    double* elem;
};

void Vector::copy(const Vector& arg)
{
    for (int i = 0; i < arg.sz; ++i)
        elem[i] = arg.elem[i];
}

Vector& Vector::operator=(const Vector& v)
{
    double* p = new double[v.sz];  // allocate new space
    copy(v);      // copy elements
    delete[] elem; // deallocate old space
    elem = p;       // no we can reset elem
    sz = v.sz;
    return *this;   // return a self-reference
}

    Jetzt kommt der Teil, den ich nicht verstehe:
    1. Neues double array auf dem heap allozieren mit der gleichen Größe wie das von v. OK
    2. Kopieren der Elemente von v in elem. Moment mal. Was, wenn v mehr elemente hat?
    3. delete[] elem. Was? Hä? Ich hab die Elemente von v nach elem kopiert, jetzt lösch ich sie plötzlich.
    4. elem = p. Toll. elem hat zwar die richtige Größe, aber steht voll lauter Müll Werten.
    5. OK, sz-Member noch anpassen, klar.
    6. Jo, standard bei Copy-assignment

    So, ich hab den Code mal geschildert, wie ich ihn verstanden hab. Wo ist mein Fehler? Glaub nicht, dass da was im Buch falsch gemacht wird.

    Meine intuitive Variante wäre so:

    Vector& Vector::operator=(const Vector& v)
{
    delete[] elem;
    elem = new double[v.sz];
    sz = v.sz;
    copy(v);
    return *this;
}

    Allerdings steht da schon im Buch, man soll nicht Information wegschmeißen, wenn man sich nicht sicher ist, dass man sie ersetzen kann. Außerdem würde das interessant werden, wenn man einen Vector sich selbst zuweist, etwa so:

    Vector v(10);
v = v;

    Also denke ich sollte das eigentlich so aussehen:

    {
    double* p = new double[v.sz];  // allocate new space
    for (int i = 0; i < v.sz; ++i)     // copy elements
        p[i] = v.elem[i];
    delete[] elem; // deallocate old space
    elem = p;       // no we can reset elem
    sz = v.sz;
    return *this;   // return a self-reference
}

    Ich hoffe jemand kann mir erklären, wo der Fehler in meinem Gedankengang ist. Schon verwirrend irgendwie.

    LG

    0
  • B

    HarteWare schrieb:

    Glaub nicht, dass da was im Buch falsch gemacht wird.

    Wieso nicht? Das ist doch glasklar Quatsch, und das hast du auch erkannt. Anstelle von copy(v) gehört da das Kopieren von v nach p hin.

    0

  • Bashar schrieb:

    HarteWare schrieb:

    Glaub nicht, dass da was im Buch falsch gemacht wird.

    Wieso nicht? Das ist doch glasklar Quatsch, und das hast du auch erkannt. Anstelle von copy(v) gehört da das Kopieren von v nach p hin.

    naja ich bin halt sehr vorsichtig und such den Fehler erstmal bei mir. Ist schon zu oft vorgekommen, dass ich dachte jemand macht Mist und dabei bin ich selber blöd.

    Da fällt mir grad ein Thread von mir ein, da hab ich mich beschwert, dass der * Operator nicht + macht.

    0
  • B

    Das ist ja OK, aber wenn es nicht um Stilfragen, sondern um logisch begründbares Richtig oder Falsch geht, kann man schonmal das Selbstbewusstsein aufbringen, die Autorität in Frage zu stellen 😉

    0
  • ?

    In der Praxis macht man das dann so:

    Vector& Vector::operator=(Vector v) { // Kopie schon bei Argumentübergabe
  std::swap(sz, v.sz);      // oder gleich swap(v);
  std::swap(elem, v.elem);  // und dann eine swap-Funktion selber implementieren
  return *this;
}

    (Copy&Swap-Idiom.)

    0
  • ?

    Schreibt man das mit c++11 nicht so?

    class Foo
{
public:
    Foo& operator=( const Foo& foo ) = default;
}
    0
  • T

    Ja, die copy() Funktion ist fehlerhaft bzw. funktioniert nur korrekt, wenn beide Vektoren gleich lang sind und kopiert die Daten auch noch in das Array, dass nach dem Funktionsaufruf frei gegeben wird.

    Deine intuitive Lösung hat zwei Probleme:
    - wenn der new Ausdruck eine Ausnahme wirft, bekommst Du das Objekt nicht mehr in einen gültigen Zustand (bestenfalls kannst Du die Vektorlänge auf 0 setzen).
    - Wenn Du einem Vektor sich selbst zuweist, greifst Du beim Kopieren auf bereits frei gegebenen Speicher zu.

    Die einfacher Variante, den Zuweisungsoperator zu implementieren ist folgende:

    Vector& Vector::operator( Vector rhs )
{
  swap( rhs );
  return *this;
}

    Das setzt dann voraus, dass man ein swap() effektiv und ohne Fehlerquelle implementieren kann:

    void Vector::swap( Vector& other )
{
  std::swap( elem, other.elem );
  std::swap( sz, other.sz );
}

    Der Copy-c'tor kann werfen, swap nicht. rhs ist eine Kopie, wie die angelegt werden konnte, dann kann swap() nicht mehr fehlschlagen und der Destruktor von temp gibt den ursprünglich allozierten Speicher wieder frei.

    Ich würde das Buch wegwerfen.

    mfg Torsten

    0
  • T

    C++11 schrieb:

    Schreibt man das mit c++11 nicht so?

    class Foo
{
public:
    Foo& operator=( const Foo& foo ) = default;
}

    Nein, die default Implementierung funktioniert an der Stelle ja eben nicht. Sie würde nur den Zeiger und den Zähler zuweisen, mit dem Effekt, dass die Adresse des einen Speicherbereichs verloren gegangen ist (memory leak) und auf den anderen Speicherbereich zwei Zeiger zeigen, und der Speicher damit zwei mal freigegeben wird (viel Spaß bei der Fehlersuche).

    0

  • Hallo Torsten,

    bei

    Vector& Vector::operator(Vector rhs)

    muss aber dann der Kopierkonstruktor (copy constructor) auch richtig implementiert sein (und nicht der Default genommen werden).
    Nach der "Regel der großen Drei" ist das zwar klar, sollte aber hier nochmals extra erwähnt werden.

    0 

  • Vector& Vector::operator=(Vector v) { // Kopie schon bei Argumentübergabe
  std::swap(sz, v.sz);      // oder gleich swap(v);
  std::swap(elem, v.elem);  // und dann eine swap-Funktion selber implementieren
  return *this;
}

    Wird hier denn 'elem' von v nach dem swap automatisch deleted?

    0
  • T

    HarteWare schrieb:

    Wird hier denn 'elem' von v nach dem swap automatisch deleted?

    Das delete sollte im Destruktor stehen und da v am Ende der Funktion "out of scope" geht, wird der Destruktor aufgerufen.

    0

  • Torsten Robitzki schrieb:

    HarteWare schrieb:

    Wird hier denn 'elem' von v nach dem swap automatisch deleted?

    Das delete sollte im Destruktor stehen und da v am Ende der Funktion "out of scope" geht, wird der Destruktor aufgerufen.

    Achja klar, das hatte mir gedanklich gefehlt..

    0
Anmelden zum Antworten