Class CResourceString

[-King-]

Ich bin ja nun nicht so der mit dem ++, aber ich möchte doch noch einige Anmerkungen zur Klasse loswerden:

Als erstes fällt der doppelte Code auf. Wenn Du jetzt was ändern willst, mußt Du immer zwei Stellen anfassen. Das ich für nicht ganz so glücklich. Besser ist es, wenn die Methode mit den wenigen Parametern einfach die Methode mit den vielen Parametern aufruft. Die eine LoadString-Methode lässt sich damit deutlich verkürzen:

PTCHAR CResourceString::LoadString(UINT uID) 
{ 
    return LoadString(m_hInstance, uID);
}

In der anderen LoadString-Methode fällt als erstes die Prüfung von m_pString auf. Das ist überflüssig, delete kannst Du auch mit NULL aufrufen. Das wird dann da überprüft. Danach Nullst Du die Variable aber nicht. Wenn jetzt das erste der folgenden news eine Exception wirft, wirst Du im Destruktor delete auf einen ungültigen Zeiger ausführen.

Außerdem würde ich auf das erste new direkt verzichten und stattdessen ein Character-Array auf dem Stack verwenden wollen.

Du hast im Grossen und Ganzen darauf geachtet, daß sich das alles auch mit UNICODE komilieren lässt. Beim Aufruf von memcpy kopierst Du aber nur die Hälfte, da Du die Multiplikation mit sizeof(TCHAR) vergessen hast.

Ah, und der Aufruf von _tcslen ist auch überflüssig, da ::LoadString die Länge des Strings bereits zurückgibt. Das ist irgendwie doppelt gemoppelt. Also eher ungefähr so (ungetestet):

PTCHAR CResourceString::LoadString(HINSTANCE hInstance, UINT uID) 
{ 
  TCHAR szTmp[MAX_BUFFERSIZE]; 
  int nLen = ::LoadString(hInstance,uID,szTmp,MAX_BUFFERSIZE); 

  TCHAR* pTmp = new TCHAR[nLen  + 1];
  delete [] m_pString;

  m_pString = pTmp;  
  m_iLength = nLen;
  _tcscpy(m_pString, szTmp); // ohne memcpy, passt immer

  return(m_pString);
}

Und wenn Du das so machst, kannst Du auch die Methode Length vereinfachen:

INT CResourceString::Length() 
{ 
  return m_iLength;
}

Und mit dem oben gesagten vereinfacht sich auch Destruktor:

CResourceString::~CResourceString() 
{ 
  delete[] m_pString; 
}

So, und nun könnt Ihr mich auseinaderreissen ...

[Aziz]

So, und nun könnt Ihr mich auseinaderreissen ...

Dich zerreißen? Ich glaube, nachdem du mich mit deiner positiven Kritik in kleinste Stücke zerrissen hast, wird niemand das selbe an dir wagen

Du hast tatsächlich sehr gute Hinweise gegeben. Herzlichen Dank dafür. Ich werde diese mal gleich in die Tat umsetzen, und wenn ich fertig bin, den Code wieder updaten.

[Aziz]

Also ich habe deinen Code genommen und ein paar Verbesserungen vorgenommen:
2 temporäre Variablen weniger, und ich verwende die memcpy Funktion. Das deshalb weil ich denke, dass sie bestimmt schneller ist als die _tcscpy Funktion, zwar sicherlich nicht beträchtlich, aber immerhin. Das Gefühl hab ich zumindest, oder was meinst du?

[-King-]

Aziz schrieb:

2 temporäre Variablen weniger, und ich verwende die memcpy Funktion.

Bleibt das Problem, wenn Dir der Speicher ausgeht. Wenn new eine Exception wirft, hast Du m_pString freigegeben. Im Destruktor gibst Du m_pString dann ein zweites mal frei. Auch passt m_iLength nicht mehr. Durch die Verwendung der beiden temporären Variablen werden die Member erst dann geändert, wenn alles geklappt hat: Problem gelöst.

Das deshalb weil ich denke, dass sie bestimmt schneller ist als die _tcscpy Funktion,

Ist auch schneller, ja. Nur schleicht sich schnell mal ein Fehler ein, wie Du gerade selbst gemerkt hast.

[Aziz]

Hab den Code wieder geändert; hab aber trotzdem noch Fragen, weil ich nicht ganz verstehe wieso man die temporären Variablen braucht:

Wenn new eine Exception wirft, hast Du m_pString freigegeben. Im Destruktor gibst Du m_pString dann ein zweites mal frei.

Ja aber wenn new keinen Speicher reservieren konnte, gibt der dann nicht NULL zurück? Der Destruktor würde dann NULL löschen, was ja gemäß deiner Aussage kein Problem ergeben sollte...

Ist auch schneller, ja. Nur schleicht sich schnell mal ein Fehler ein, wie Du gerade selbst gemerkt hast.

Naja, das ist eben nur mir passiert, weil ich zu unüberlegt programmiert habe

[.]

Schreib doch besser eine Funktionen die einen std::string zurückgibt. Das fände ich sinnvoller als diese Klasse.

[-King-]

Aziz schrieb:

Ja aber wenn new keinen Speicher reservieren konnte, gibt der dann nicht NULL zurück?

So, wie es da steht, nicht. Und selbst wenn: Du prüfst das nicht. In diesem Falle müsstest Du m_iLength auf 0 zurücksetzen.

[Aziz]

So, wie es da steht, nicht. Und selbst wenn: Du prüfst das nicht. In diesem Falle müsstest Du m_iLength auf 0 zurücksetzen.

Wieso nicht? Ein Zitat aus der MSDN:

If there is insufficient memory for the allocation request, by default operator new returns NULL.

Wenn kein Speicher vorhanden ist sollte immer NULL zurückgegeben werden. Die Sache mit m_iLength kann man ja noch erledigen...

@"."

Schreib doch besser eine Funktionen die einen std::string zurückgibt. Das fände ich sinnvoller als diese Klasse.

Ich denke das ist unnötig, zumindest hinsichtlich meiner Bedürfnisse. Wenn alles bei den elementaren Funktionen und Datentypen bleibt ist es auch schneller. Gibt es außerdem eine Unicode version von string?

[-King-]

Bei mir steht das anders:

The default behavior of a new handler is to throw an object of type bad_alloc.

[Aziz]

Da steht "a new handler" und nicht "the new operator". Man kann mittels der _set_new_handler Funktion einen Handler festlegen, der aufgerufen wird wenn der new Operator beim Allozieren von Speicher versagt...

[-King-]

Ok, ich geb's auf. Das Standard-Verhalten ist jedenfalls anders als von Dir gewünscht.

[Aziz]

Ich würd' gern wissen was dich da so sicher macht..

Einerseits sagt die MSDN (die Version die ich installiert hab) klar aus, dass der new Operator sich auf diese Weise verhält, und auf der anderen Seite drückt sich die MSDN im Internet etwas unklar darüber aus..

[-King-]

Aziz schrieb:

Ich würd' gern wissen was dich da so sicher macht..

Soweit ich weiß, soll laut Standard im Fehlerfall eine Exception geschmissen werden. Meine Umgebung sorgt per default dafür, eben so, wie es in der zugehörigen Dokumentation steht.

Wenn Du da irgendetwas altes installiert hast, kann das auch durchaus anders aussehen. Allerdings sollte das eher als Bug durchgehen.

Notfalls kannst Du im C++ - Forum nachfragen. Die können Dir das genaue Verhalten von new besser erkären als ich es kann. Schliesslich bin ich, wie Eingangs erwähnt, eher der Typ ohne ++.

[Aziz]

Hmm, mit dem C++ Standard kenn ich mich minimal aus. Ich verwende die Visual Studio SP 5.0 Entwicklungsumgebung; welche verwendest du?

Ich werd vielleicht im C++ Forum nachfragen wegen der "new" Sache..

[-King-]

VC6? Dann hast Du natürlich recht, bei diesem alten Ding war das noch so. Ich verwende die aktuelle Version 7.1, die verhält sich richtig.

[Aziz]

-King- schrieb:

VC6? Dann hast Du natürlich recht, bei diesem alten Ding war das noch so. Ich verwende die aktuelle Version 7.1, die verhält sich richtig.

Ich hab da was Interessantes gefunden: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vclang/html/_pluslang_the_new_and_delete_operators.asp

Man kann dieses Verhalten anscheinend auch im VC6 erzielen.