Referenz als Rückgabewert

WebFritzi

Hi,
ich schreibe gerade zur Übung eine einfache Klasse, die die Stringfunktionen der C-Laufzeitbibliothek kapseln soll. Jau, und da wollt ich eine Memberfunktion "ToUpper()" machen, die eine Instanz meiner Klasse zurückgibt, die den String in Großbuchstaben enthält. Ich möchte also sowas machen können:

{
   CString cstr1("Hallo");
   CString cstr2;

   cstr2 = cstr1.ToUpper();
}

Das habe ich auch so hinbekommen, nur weiß ich nicht, ob da vielleicht Memory-Leaks entstehen. Die entscheidende Zeile ist die dritte. ToUpper() sieht so aus:

CString& CString::ToUpper()
{
   // m_String ist eine private-Variable, in der der String
   // enthalten ist: LPTSTR m_String, wobei
   // #ifdef UNICODE
   //    #define LPTSTR char*
   // #else
   //    #define LPTSTR wchar_t*
   // #endif
   CString* str = new CString(m_String); 
   str->UpperCase();   // str in Großbuchstaben konvertieren
   return *str;
}

Und der Zuweisungsoperator so:

void CString::operator =(CString& rhs)
{
   delete[] m_String;

   LPTSTR str = rhs.c_str();
   m_String = (LPTSTR)new TCHAR[lstrlen(str) + 1];
   lstrcpy(m_String, str);
}

Ich denke, das ist selbsterklärend. Nun habe ich aber ein Problem im obigen Beispiel-Code. Nach den ersten beiden Zeilen habe ich 2 Instanzen von CString auf dem Stack. So, und in der dritten Zeile wird zuerst "cstr1.ToUpper()" ausgeführt. Das heißt, er erstellt eine neue Instanz von CString auf dem Heap, konvertiert den in dieser enthaltenen String in Großbuchstaben und gibt diese Instanz zurück. Dann wird der Zuweisungsoperator ausgeführt. cstr2 übernimmt dann den String der in der Heap-Instanz enthalten ist. Am Ende des Blocks werden die Destruktoren von cstr1 und cstr2 aufgerufen. Toll, aber was ist mit der Heap-Instanz? Die wird wohl nicht gelöscht, oder?
Was ich möchte, ist, dass ToUpper() einen CString zurückgibt, der am Ende des Blocks gelöscht wird. Ist das so OK, oder wie mache ich das?

MFK

Warum lässt du ToUpper nicht einfach const CString zurückgeben? Dann kannst du das neue Objekt in der Methode auf dem Stack erzeugen, und beim Zurückgeben wird eine Kopie angelegt.

Übrigens, wenn du deinen op= so aufziehst (also erst delete, dann new), dann musst du auf Selbstzuweisung prüfen.

volkard

WebFritzi schrieb:

...Das habe ich auch so hinbekommen, nur weiß ich nicht, ob da vielleicht Memory-Leaks entstehen.

messen hilft. mach mal

for(;;)
{ 
   CString cstr1("Hallo"); 
   CString cstr2; 

   cstr2 = cstr1.ToUpper(); 
}

und starte das. und schau auf den verlauf der auslagerungsdateiauslastung (stg+alt+enf, systemleistung). die dicken löcher findet man so recht schnell.

denke nicht, daß es irgendwie sinnvoll sein kann, hier an ne referenz oder nen zeiger als rückgabewert zu denken.

CString& CString::ToUpper()
{
   CString str(m_String);
   str->UpperCase();// str in Großbuchstaben konvertieren
   return str;
}

Gerard

ich würde entweder ToUpper oder UpperCase anbieten, halte die schnittstelle überschaubar

WebFritzi

@MFK: (wegen delete und new) Ich wende "delete" ja nicht auf meine Instanz an, sondern auf einen Member.

volkard schrieb:

denke nicht, daß es irgendwie sinnvoll sein kann, hier an ne referenz oder nen zeiger als rückgabewert zu denken
CString& CString::ToUpper()
{
   CString str(m_String);
   str->UpperCase();// str in Großbuchstaben konvertieren
   return str;
}

Das kapiere ich jetzt ned mehr. OK, Frage: Was ist der Unterschied zwischen

int Dec(int a)
{
   int r = a + 1;
   return r;
}

und

int& Dec(int a)
{
   int r = a + 1;
   return r;
}

?

Jester

Der Unterschied ist, daß lezteres ín undefiniertem Verhalten resultiert, weil hier eine Referenz auf eine lokale Variable zurückgegeben wird.

Übrigens hat Dein operator= ein Problem mit Selbstzuweisung, da wird nämlich der Speicher gelöscht... und anschließend kopiert. Nur wenn das der gleiche Speicher ist, der zuvor freigegeben wurde ist das nicht so günstig.

WebFritzi

Jester schrieb:

Der Unterschied ist, daß lezteres ín undefiniertem Verhalten resultiert, weil hier eine Referenz auf eine lokale Variable zurückgegeben wird.

Und warum gibt Volkard hier dann genau solch ein Beispiel? Schau auf seinen letzten Code (in dem natürlich '->' ein '.' sein muss).

Jester schrieb:

Übrigens hat Dein operator= ein Problem mit Selbstzuweisung, da wird nämlich der Speicher gelöscht... und anschließend kopiert. Nur wenn das der gleiche Speicher ist, der zuvor freigegeben wurde ist das nicht so günstig.

Hä? Ich lösche doch nur einen (Member-)String (also einen Pointer) in meiner Klasse und weise ihm wieder neuen Speicher zu. Was ist daran verkehrt?

Weil Volkard ein Noob ist, denke ich.

WebFritzi

Dann kennste Volkard nicht!

davie

stell dir vor ein:

CString sinnlos("aber dumm");
sinnlos = sinnlos;

sinnlos löscht jetzt den speicher, allokiert stattdessen neuen
und? es kopiert den neuen speicher in den neuen (der alte ist ja schon gelöscht)

WebFritzi

I see. DAS meint ihr mit Selbstzuweisung. Alles klar. Also müsste ich noch sowas schreiben wie

if(&rhs != this)

oder

if(rhs.c_str() != m_String)

Gut, diese Frage ist beantwortet. Aber meine eigentliche Frage noch nicht. Wie kann ich es also hinbekommen, dass sich folgender Code kompilieren lässt UND (vor allem) dass er auch Sinn macht? Die Frage bezieht sih auf die ToUpper()-Funktion.

CString cstr1("Hallo"); 
CString cstr2; 

cstr2 = cstr1.ToUpper();

Und jetzt geht bitte davon aus, dass der Zuweisungsoperator ordentlich implementiert ist.

Bashar

indem du ToUpper einen Wert zurückgeben läßt. Geht leider nicht besser, wenn du deinen Code nicht ändern willst, da der Compiler nicht intelligent genug ist bzw. nicht sein darf.

man könnte aber z.b. erstens cstr2 gleich initialisieren, um return value optimization auszunutzen:

CString cstr2 = cstr1.ToUpper();

oder man könnte eine swap-Methode implementieren, die die Repräsentationen der Strings intern vertauscht, und dann so schreiben:

CString cstr2;
cstr2.swap(cstr1.ToUpper());

Das erledigt gleich das im Grunde unnötige Kopieren im Zuweisungsoperator. Eigentlich ist der Compiler selbst intelligent genug, zu erkennen, dass du das temporäre Ergebnis von ToUpper nach der Zuweisung nicht mehr benötigst, und daher auch keine Kopie gemacht werden muss. Diese Optimierung verändert aber die Semantik des Programms, wenn dein Zuweisungsoperator Seiteneffekte hat, und darf daher nicht gemacht werden.

Ich hoffe ich habe nicht völligen Blödsinn geschrieben

Jester

Dieso swap-Methode halte ich sowieso für sinnvoll.

Dann wird der operator= nämlich mit

CString temp(rhs);
swap(*this, rhs);

vollständig implementiert. Das Problem der Selbstzuweisung ist auch keines mehr und Exception.sicher ist es auch noch, sofern swap so implementiert ist, daß es nie wirft.

Im anderen Fall ist nämlich die Frage was passiert, wenn die aktuelle Resource zwar mit delete [] freigegeben wird, beim erneuten anfordern aber bad_alloc fliegt... alte Daten weg, neue Daten gibts nich => inkonsistenter Zustand, Objekt kaputt.

DrGreenthumb

Bashar schrieb:

man könnte aber z.b. erstens cstr2 gleich initialisieren, um return value optimization auszunutzen:
CString cstr2 = cstr1.ToUpper();

Hm, heißt das nrvo funktioniert nur bei Initialisierungen?

Also bei

string k;
k = func();

kann der compiler nicht mehr optimieren?

davie

Bashar schrieb:

CString cstr2;
cstr2.swap(cstr1.ToUpper());

und jetzt erklär noch mal den unterschied zu

CString cstr2;
cstr2.operator=(cstr1.ToUpper());

holla. steh ich auf der leitung?

Bashar

DrGreenthumb schrieb:

Hm, heißt das nrvo funktioniert nur bei Initialisierungen?

Bei NRVO wird intern der Funktion ein Zeiger auf das Zielobjekt übergeben und das zurückgegebene Objekt direkt dort konstruiert. Bei Initialisierung klappt das wunderbar, aber bei Zuweisung nicht mehr: Der Zuweisungsoperator ist schließlich was anderes als der Kopierkonstruktor.
Also wird erstmal ein temporäres Objekt erzeugt, auf welches dem Zuweisungsoperator eine Referenz übergeben wird. Da innendrin wird dann nochmal kopiert (wenn wir copy-on-write mal ausschließen)

Da kommt jetzt davie's Einwand ins Spiel: Mit swap wird eben nicht kopiert. Die alte leere Hülle von cstr2 -- eh nur default-konstruiert -- wird beim Entsorgen des temporären Objekts weggeworfen. cstr2 übernimmt die interne Repräsentation des von ToUpper zurückgegebenen Objektes.

Der Zuweisungsoperator kann nicht davon ausgehen, dass die rechte Seite nicht mehr gebraucht wird, muss also immer eine richtige Kopie machen (wenn wir COW wieder ausschließen)

davie

menno.. sollte schlafen gehen. n8

DrGreenthumb

Danke, dachte die linke Seite wird einfach als Parameter mitgegeben. Wusste nicht, das dass so kompliziert ist.. Aber wenn ich drüber nachdenke eigentlich logisch, das Objekt muss ja komplett neu konstruiert werden.

WebFritzi

OK, dann hab ich jetzt noch ne Frage dazu:

int Dec(int a)
{
   int r = a + 1;
   return r;
}

int main()
{
   int f = 1;
   int g;
   g = Dec(f);
   ...
}

Was passiert hier? Ist folgendes richtig?

1. Es wird ein int f auf dem Stack reserviert und mit dem Wert 1 gefüllt
2. Es wird ein int g auf dem Stack reserviert
3. Es wird ein int r auf dem Stack reserviert und mit dem Wert 2 gefüllt
4. Der Wert von r wird zu g [b]rüberkopiert[/b], so dass g mit 2 gefüllt wird
5. Der Speicher, den r belegte, wird wieder freigegeben

Zweite Frage: Was bedeutet const am Ende einer Funktionsdeklaration?

CString CString::ToUpper() const;

fubar

zu Frage 2:
Dieses const bedeutet IMHO, daß der Compiler den this-Zeiger als const this* an die Elementfunktion übergibt, d.h., die Funktion kann die Elemente der Klasse nicht verändern.

Bitte korrigiert mich, wenn ich ich Unsinn erzähle, ist noch früh *gähn*