const return bei operator =

Seikilos

Ich hab eine Aufgabe, die ich hoffentlich richtig verstehe:

Overload = operator to assign the Polygon object to another. This member
function should return const reference to the Polygon object invoking the
member function. This member function should also test for self assignment.

Dh doch, dass die Zuweisung keine Kopie sondern eine const referenz ist? Aber was heißt dass, dass ich die linke Seite nicht mehr modifizieren kann?
Ich hab es so probiert:

const Polygon& Polygon::operator =(const Polygon & p){
	if(this != &p){
		// NOT SELF

                // char arrays
		xPts = p.xPts;
		yPts = p.yPts;

		numberOfVertices = p.numberOfVertices;

	}

	return *this;

}

Ich verweise also direkt die Pointer von dem alten Objekt aufs neue, soweit ok und returne *this, aber müsste dass dann nicht const sein? Wenn ich sowas habe wie:

Polygon p1,p2;
p2 = p1;
p2.operation(); <--- illegal?

Bei mir kann ich P2 dennoch fröhlich nutzen

LordJaxom

p2 ist auch nicht konstant, Du hast es ja eine Zeile darüber erst als nichtkonstant definiert. Nur der Rückgabewert von (p2 = p1) ist konstant.

Seikilos

Hmm, was bedeutet es denn, wenn der Rückgabewert konstant ist? Und verstehe ich die Aufgabenstellung richtig?

CStoll

Du mußt unterscheiden zwischen den internen Abläufen im Zuweisungsoperator (kopieren der Member) und dem Rückgabewert (der wird z.B. verwendet, wenn die Zuweisung im Inneren eines größeren Ausdrucks passiert - im einfachsten Fall eine Mehrfachzuweisung "p3=p2=p1;").

PS: So, wie du den Operator jetzt definiert hast, ist er auch nicht besser als der automatisch generierte. (btw, was für einen Typ haben xPts und yPts - und wie agieren Copy-Ctor und Dtor mit ihnen?)

Seikilos

Polygon ist eine Klasse, die zwei int Arrays (wie komme ich auf char arrays) verwaltet. Nun steht halt oben die Aufgaben beschreibung und ich hab versucht, die umzusetzen, so wie es gemeint ist. Dass das ding meiner Meinung nach auch nichts bewirkt dachte ich auch.
Dtor deleted [] die beiden, der Copy Ctor erstellt eine per Hand kopie der Werte und der Ctor initialisiert die mit einer Zahl (Tiefe des Arrays, hier weggelassen)

So ganz verstehe ich das Const aber nicht, auch bei mehrfachzuweisung nicht

LordJaxom

Seikilos schrieb:

Dtor deleted [] die beiden, der Copy Ctor erstellt eine per Hand kopie der Werte und der Ctor initialisiert die mit einer Zahl (Tiefe des Arrays, hier weggelassen)

Dann ist hier der Punkt um anzusetzen. Ich denke schon dass in der Aufgabenstellung genug Mitdenken vorausgesetzt wird um zu meinen "ein Polygon sinnvoll einem anderen zuweisen". Um zu ermitteln was zu "sinnvoll" noch fehlt, schaue Dir doch mal an was passiert wenn Du im momentanen Zustand ein Polygon konstruierst, ein zweites aus dem ersten kopierst (Copy-Ctor) und beide dann löscht.

CStoll

Seikilos schrieb:

Polygon ist eine Klasse, die zwei int Arrays (wie komme ich auf char arrays) verwaltet. Nun steht halt oben die Aufgaben beschreibung und ich hab versucht, die umzusetzen, so wie es gemeint ist. Dass das ding meiner Meinung nach auch nichts bewirkt dachte ich auch.
Dtor deleted [] die beiden, der Copy Ctor erstellt eine per Hand kopie der Werte und der Ctor initialisiert die mit einer Zahl (Tiefe des Arrays, hier weggelassen)

Und im Zusammenspiel damit ist dein Zuweisungsoperator eine mittlere Katastrophe - auf der einen Seite hast du ein Speicherleck für die alten Daten des Ziels, auf der anderen Seite versuchst du später zweifach, die neu zugewiesenen Daten zu löschen (im Dtor beider beteiligten Polygone). Korrekterweise müsstest du dort die bisherigen Polygondaten delete[]n und anschließend die Daten des Quellpolygons in einen eigenen Datenblock kopieren.
(wenn der Platz ausreicht, kannst du die Array-Inhalte auch einfach kopieren)

So ganz verstehe ich das Const aber nicht, auch bei mehrfachzuweisung nicht

In größeren Ausdrücken wird der Rückgabewert des Operators weitergegeben - "p3=p2=p1;" wird z.B. aufgelöst zu "p3.operator=(p2.operator=(p1));". Indem der Rückgabetyp const gesetzt wird, verhinderst du, daß jemand schreibend über diesen Wert hantiert, z.B. per "(p2=p1).operation();".

Seikilos

Ja, da kommt nichts gutes bei rum, weil der Speicher beim ersten löschen schon frei gegeben wird.

Dann also kopieren, war auch meine erste idee. Ich hab ja auch einen Copy Ctor, der beide arrays kopiert.
Mein erster Ansatz dafür war auch sowas in die Richtung:

const Polygon& Polygon::operator =(const Polygon & p){
    if(this != &p){
        // NOT SELF

        delete [] xPts;
        delete [] yPts;

        numberOfVertices = p.numberOfVertices;

        // Per hand die beiden Arrays kopieren

    }

    return *this;

}

Da fand ich die redundanz aber doof, weil ich ja das kopieren schon im copy ctor umgesetzt habe. Daraus wollt ich nach meiner Logik dies machen:

const Polygon& Polygon::operator =(const Polygon & p){
    if(this != &p){
        // NOT SELF

        delete [] xPts;
        delete [] yPts;

        numberOfVertices = p.numberOfVertices;

        // Copy Ctor nutzen
        *this = Polygon(p);  // mein copy ctor nimmt ja Polygon & als Param

    }

    return *this;

}

Da meckert er aber,'<function-style-cast>': 'const Polygon' kann nicht in 'Polygon' konvertiert werden.
Heißt das ich hätte meinen Copy Ctor als const Polygon & definieren sollen?

Edit: Hab den Copy Ctor als const Polygon & definiert, nun knallt die Stelle *this = Polygon(p); mit ner Exception bad_alloc raus

Edit2: Es klappt natürlich, wenn ich ein temp Objekt erstelle und alle wichtigen Variablen mit std::swap wechsel, aber mich interessiert, warum ich das da oben nicht machen kann

CStoll

Seikilos schrieb:

Da meckert er aber,'<function-style-cast>': 'const Polygon' kann nicht in 'Polygon' konvertiert werden.
Heißt das ich hätte meinen Copy Ctor als const Polygon & definieren sollen?

Erstens das (wie ist denn die Signatur des Copy-Ctor's?) und zweitens hast du hier eine erstklassische Enlosrekursion eingebaut - "*this=Polygon(p);" ruft wieder erst den Copy-Ctor auf, um ein temporäres Polygon anzulegen - und anschließend operator=, um dieses temporäre Objekt nach this zu kopieren.

Wenn du Redundanzen vermeiden willst, dann nimm lieber den Copy-and-Swap Ansatz:

const Polygon& Polygon::operator =(const Polygon & p){
    if(this != &p){
        // kopiere die Quelle in ein temporäres Objekt
        Polygon tmp(p);
        // vertausche Daten von *this und tmp
        // (das kannst du auch in eine eigene Funktion auslagern
        swap(xPts,tmp.xPts);
        swap(yPts,tmp.yPts);
        swap(numberOfVertices,tmp.numberOfVertices);
        // am Blockende wird tmp beseitigt und gibt seine Daten (die alten Daten von *this) wieder frei
    }

    return *this;

}

(alternativ kannst du anstelle der nackten Arrays auch std::vector<> verwenden - der kümmert sich um alles Notwendige von alleine)

Seikilos

Danke
Mir war nicht klar, dass das eine Endlosrekursion ist und das dort wieder der = Operator benutzt werden würde

pumuckl

du kannst den Copy-Ctor nicht so ohne weiteres fuer ein bestehendes Objekt benutzen. Der Ctor ist naemlich nicht nur zum kopieren da, sondern auch um zu initialisieren, was im Zuweisungsoperator nicht mehr noetig ist. Was moeglich waere, ist, die Kopiersemantik, die in beiden Funktionen vorkommt, in eine (private) extrafunktion zu verlegen:

struct Polygon {
/*...*/

  Polygon(const Polygon& p) {
    copydata(p);
  }

  const Polygon& operator=(const Polygon& rhs) {
    if (this != &rhs) {
      delete[] xPts;
      delete[] yPts;
      copydata(rhs);
    }
    return *this;
  }

private:
  void copydata(const Polygon& p) {
    numberOfVertices = p.numberOfVertices;
    xPts = new int[numberOfVertices];
    yPts = new int[numberOfVertices];
    std::copy_n(p.xPts, numberOfVertices, xPts);
    std::copy_n(p.yPts, numberOfVertices, yPts);
  }

  int* xPts;
  int* yPts;
  size_t numberOFVertices;
/*...*/
};