do{}while(false); -- ein besseres goto?
-
DrGreenthumb schrieb:
;fricky schrieb:
ich finde es doof extra eine funktion 'avoid_goto()' zu schreiben, die aus dem einzigen grund da ist, um ein goto zu vermeiden.
wenn du die funktion nicht avoid_goto nennen würdest sondern danach, was sie macht, hätte sie zusätzlich auch noch den effekt, den code besser zu dokumentieren.
da habt ihr natürlich recht. trotzdem widerstrebts mir, code der sonst eine einheit darstellt zu zerstückeln, nur um ein harmloses 'goto' zu umgehen. wenn die schleifendurchläufe dokumentationswürdig sind, kann man auch so einen kurzen kommentar hinschreiben. bei so'ner extrahierten anti-goto funktion, brauchste womöglich noch 'nen kommentar wie */* don't call it directly, it's fucking useless /
-
;fricky schrieb:
DrGreenthumb schrieb:
;fricky schrieb:
ich finde es doof extra eine funktion 'avoid_goto()' zu schreiben, die aus dem einzigen grund da ist, um ein goto zu vermeiden.
wenn du die funktion nicht avoid_goto nennen würdest sondern danach, was sie macht, hätte sie zusätzlich auch noch den effekt, den code besser zu dokumentieren.
da habt ihr natürlich recht. trotzdem widerstrebts mir, code der sonst eine einheit darstellt zu zerstückeln, nur um ein harmloses 'goto' zu umgehen. wenn die schleifendurchläufe dokumentationswürdig sind, kann man auch so einen kurzen kommentar hinschreiben. bei so'ner extrahierten anti-goto funktion, brauchste womöglich noch 'nen kommentar wie */* don't call it directly, it's fucking useless /
Es tauchen keine Funktionen auf, die so einen Kommentar bräuchten. Die Suche und die Verarbeitung des gefundenen Wertes sind eben NICHT eine Einheit.
-
volkard schrieb:
Es tauchen keine Funktionen auf, die so einen Kommentar bräuchten. Die Suche und die Verarbeitung des gefundenen Wertes sind eben NICHT eine Einheit.
jetzt pauschalisierste aber. was immer in den schleifen passiert, es kann so speziell sein, dass es an anderer stelle absolut nutzlos wäre.
-
volkard schrieb:
;fricky schrieb:
volkard schrieb:
Ich sehe da zwei verschiedene Aussagen.
Es gibt keinen angemessenen Einsatz von goto.
Jup. Für C++ mag das stimmen.
in C++ tritt man 'ne exception los und fängt sie dahinter wieder auf, ne? (ich meine jetzt wenn man aus den verschachtelten schleifen raus will).
Nein, das ist eher Java-Stil.
Na danke für die Zurschaustellung deiner Ahnug du Provi
Würd mich wundern, wenn C und C++ kein break&continue in schleifen kennen?
-
volkard is dumm schrieb:
Würd mich wundern, wenn C und C++ kein break&continue in schleifen kennen?
das bezieht sich aber nur auf die innerste schleife. in Java gibts übrigens ein 'break' mit label um verschachtelungen zu verlassen:
... nested_loops: for(...) { while (...) { if (done) break nested_loops; } } // hier gehts weiter nach dem break ...
-
;fricky schrieb:
volkard schrieb:
Es tauchen keine Funktionen auf, die so einen Kommentar bräuchten. Die Suche und die Verarbeitung des gefundenen Wertes sind eben NICHT eine Einheit.
jetzt pauschalisierste aber. was immer in den schleifen passiert, es kann so speziell sein, dass es an anderer stelle absolut nutzlos wäre.
Beispiel?
-
;fricky schrieb:
volkard schrieb:
Ich sehe da zwei verschiedene Aussagen.
Es gibt keinen angemessenen Einsatz von goto.
Jup. Für C++ mag das stimmen.
in C++ tritt man 'ne exception los und fängt sie dahinter wieder auf, ne? (ich meine jetzt wenn man aus den verschachtelten schleifen raus will).
Man wirft in C++ oft eine Exception, und fängt die *nicht* wieder auf, wo man in C
"goto exit;"verwenden würde. Oder macht ganz einfach"return;"wo man in C"goto exit;"machen würde.Anders gesagt: der Haupt-Einsatzzweck von
"goto"in C ist IMO das Springen zum "Cleanup-Teil" einer Funktion (Files zumachen, Speicher freigeben etc.). Und genau das kann man in C++ besser lösen, ohne"goto".
-
;fricky schrieb:
DStefan schrieb:
Bin halt extrem goto-geschädigt.
was ist passiert mit dir und 'goto'?
Ich bin goto-geschädigt, weil ich vor Jahren in ein Projekt kam, das schon knapp zwei Jahre lief und (teilweise) vor gotos geradezu wimmelte. Das war meist richtig guter Code aber eben fast überall so ne Art "C-Mit-Klassen".
Wir haben das Projekt gepflegt, erweitert und später nach NT portiert. Und in dieser Zeit habe ich ziemlich gründlich gelernt, dass gotos im Original-Code vielleicht gar nichts böses tun. Wenn man aber an dem Code arbeiten muss, bringts einen um.
Nimm einfach nur sowas (ich weiß, das ist sehr primitiv!):
foo() { // ... if(condition) goto done // ... done: // ... }Jetzt musst du was ändern und der natürlichste Weg wäre:
foo() { // ... if(condition) goto done // ... MyClass myObject; myObject.machwas(); // ... done: // ... }Das geht nicht, weil das goto am Ctor von myObject vorbei springt.
Letztlich läuft es darauf hinaus, dass man die gesamte Funktion umschreiben muss. Und meine Erfahrung war halt, dass man das verdammt oft und immer wieder musste. Zundem in Funktionen, die nicht so einfach waren wie das Beispiel.
Wir haben uns letztlich allen Ernstes geweigert, Abschätzungen für Änderungen auf der "Müllhalde" abzugeben. Weil wir einfach zu oft daneben gelegen hatten. Ok, ich geb's zu, das war nicht bloß deswegen. Aber es war sehr oft deswegen.
Und nochmal: Der ursprüngliche Code war gar nicht so schlecht. Da waren Leute mit Verstand dran. Aber er war sowas von Änderungs-resistent.....
Ich glaube, volkard und andere haben schon ziemlich gut gezeigt, dass man gotos nicht wirklich braucht. Oder wenigesten fast immer nicht wirklich braucht. Was man braucht, ist gutes C++. Ehrlich: Ich möchte immer in Projekten arbeiten, wo goto verboten ist.
Stefan.
-
Ach ja: Die do-while(false)-Lösung hat zumindeste dieses Problem nicht. Oder irre ich mich?
Stefan.
-
DStefan schrieb:
Ach ja: Die do-while(false)-Lösung hat zumindeste dieses Problem nicht. Oder irre ich mich?
Stefan.Welches Problem?
foo() { // ... if(condition) goto done // ... { MyClass myObject; myObject.machwas(); } // ... done: // ... }
-
volkard schrieb:
;fricky schrieb:
volkard schrieb:
Es tauchen keine Funktionen auf, die so einen Kommentar bräuchten. Die Suche und die Verarbeitung des gefundenen Wertes sind eben NICHT eine Einheit.
jetzt pauschalisierste aber. was immer in den schleifen passiert, es kann so speziell sein, dass es an anderer stelle absolut nutzlos wäre.
Beispiel?
z.b. wenn in den schleifen ein paar lokale variablen verändert werden, deren werte nach der schleifenkonstruktion abgefragt werden, oder sowas. etwas ganz spezielles eben, was nur dort einmal gebraucht wird. irgendwie kann man's natürlich immer in eine extra funktion auslagern, variablen als struct-pointer übergeben oder so, aber ist es das wirklich wert, nur um *einem* goto aus dem weg zu gehen?
hustbaer schrieb:
Anders gesagt: der Haupt-Einsatzzweck von
"goto"in C ist IMO das Springen zum "Cleanup-Teil" einer Funktion (Files zumachen, Speicher freigeben etc.). Und genau das kann man in C++ besser lösen, ohne"goto".naja, in C isses nicht der einzige zweck. aber was das cleanup angeht: auch in C++ wäre es vorstellbar, dass am schluss noch etwas mehr gemacht werden muss, als die destruktoren der einzelnen objekte tun. natürlich könnteste dir für den fall eine eigene klasse schreiben, mit 'nem monster-destruktor drin, aber das hört sich für mich auch nicht nach 'ner tollen idee an. wäre ich c++ progger würde ich mir dann vielleicht doch lieber selbst eine exception werfen, ...oder einfach ein kleines 'goto' einbauen.
DStefan schrieb:
Ich bin goto-geschädigt, weil ich vor Jahren in ein Projekt kam, das schon knapp zwei Jahre lief und (teilweise) vor gotos geradezu wimmelte.
klar, sowas ist doof. gotos sollte man nicht verwenden, wenn die programmiersprache bessere möglichkeiten zur strukturierung bietet (siehe z.b. das java break-statement weiter oben). nur in ausnahmefällen, wenn man was damit vereinfachen kann.
DStefan schrieb:
Was man braucht, ist gutes C++.
na, damit sprichste ja jetzt den richtigen an *grins*
-
;fricky schrieb:
volkard schrieb:
Beispiel?
z.b. wenn in den schleifen ein paar lokale variablen verändert werden, deren werte nach der schleifenkonstruktion abgefragt werden, oder sowas.
Beispiel?
-
volkard schrieb:
;fricky schrieb:
volkard schrieb:
Beispiel?
z.b. wenn in den schleifen ein paar lokale variablen verändert werden, deren werte nach der schleifenkonstruktion abgefragt werden, oder sowas.
Beispiel?
quelltext? bitte:
BOOL IE401IndexFile::EnumHashValues() { BOOL retVal = FALSE; if( m_pBuf == NULL) goto doneEnumHashValues; HASH_FILEMAP_ENTRY* pTable; HASH_ITEM* pItem; // pTable is located by an offset which is located at dwHashTableOffset pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); // The first location in the table follows immediately after the HASH_FILEMAP_ENTRY pTable pItem = (HASH_ITEM*)(pTable + 1); if (pTable->dwSig != SIG_HASH) goto doneEnumHashValues; do // Scan the list of tables. { // Scan the current table. for (; (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) { // call virtual entry handler if( HandleHashElement( pItem) == FALSE) goto doneEnumHashValues; } // Follow the link to the next table. if (!pTable->dwNext) { pTable = NULL; } else { // Validate the table signature and sequence number. DWORD nBlock; nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->dwSig != SIG_HASH || pTable->nBlock != nBlock + 1) goto doneEnumHashValues; // Set pointer to first location in table. pItem = (HASH_ITEM*) (pTable + 1); } } while (pTable); retVal = TRUE; doneEnumHashValues: return retVal; }^^stammt vom windows-nt source code, urlcache, file: 401imprt.cxx
die beiden ersten gotos ignorier mal, die müssen nicht sein, aber die in den schleifen usw. schon. auch mit den ersten gotos ist der ablauf leicht zu erkennen, also von spaghetti-gespringe noch weit entfernt.
-
;fricky schrieb:
hustbaer schrieb:
Anders gesagt: der Haupt-Einsatzzweck von
"goto"in C ist IMO das Springen zum "Cleanup-Teil" einer Funktion (Files zumachen, Speicher freigeben etc.). Und genau das kann man in C++ besser lösen, ohne"goto".naja, in C isses nicht der einzige zweck. aber was das cleanup angeht: auch in C++ wäre es vorstellbar, dass am schluss noch etwas mehr gemacht werden muss, als die destruktoren der einzelnen objekte tun. natürlich könnteste dir für den fall eine eigene klasse schreiben, mit 'nem monster-destruktor drin, aber das hört sich für mich auch nicht nach 'ner tollen idee an. wäre ich c++ progger würde ich mir dann vielleicht doch lieber selbst eine exception werfen, ...oder einfach ein kleines 'goto' einbauen.
In dem Fall verwende ich Scope-Guards. Entweder was fertiges (Boost), oder zur Not auch selbst geschriebene kleine "spezialisierte" Scope-Guards. Was dann inetwa dem entspricht was du mit "Klasse mit einem Monster-Destruktor drin" umschreibst. Finde ich persönlich grundsätzlich nicht wirklich unsauber. Natürlich kann man auch damit unsaubere, unwartbare Programme schreiben.
-
hustbaer schrieb:
In dem Fall verwende ich Scope-Guards. Entweder was fertiges (Boost), oder zur Not auch selbst geschriebene kleine "spezialisierte" Scope-Guards.
habs mir gerade angeschaut: http://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Scope_Guard
da wittere ich doch glatt einen schickes objekt für den nächsten c++ lästerthread *grins*
-
BOOL IE401IndexFile::EnumHashValues() { if( m_pBuf == NULL) return FALSE; HASH_FILEMAP_ENTRY* pTable; HASH_ITEM* pItem; // pTable is located by an offset which is located at dwHashTableOffset pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); // The first location in the table follows immediately after the HASH_FILEMAP_ENTRY pTable pItem = (HASH_ITEM*)(pTable + 1); if (pTable->dwSig != SIG_HASH) return FALSE; do // Scan the list of tables. { // Scan the current table. for (; (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) { // call virtual entry handler if( HandleHashElement( pItem) == FALSE) return FALSE; } // Follow the link to the next table. if (!pTable->dwNext) { pTable = NULL; } else { // Validate the table signature and sequence number. DWORD nBlock; nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->dwSig != SIG_HASH || pTable->nBlock != nBlock + 1) return FALSE; // Set pointer to first location in table. pItem = (HASH_ITEM*) (pTable + 1); } } while (pTable); return TRUE; };fricky schrieb:
die beiden ersten gotos ignorier mal, die müssen nicht sein, aber die in den schleifen usw. schon. auch mit den ersten gotos ist der ablauf leicht zu erkennen, also von spaghetti-gespringe noch weit entfernt.
Sehr gewagte Aussage. ZUm einen ist es relativ schwer zu erkennen das in allen Fällen ein "return FALSE" passiert ausser wenn die while-schleife komplett durchlaufen wurde. Dazu muss man erstmal raussuchen an welchen Stellen retVal überall geändert wird und dann verstehen das bei sämtlichen gotos der Wert immer FALSE ist.
Ja, es gibt Anhänger des Glaubens das mehrfache returns aus einer Funktion böse sind... so what. Es gab auch Leute die daran geglaubt haben die Erde sei eine Scheibe und zu gewissen Zeiten war das sogar die überwiegende Mehrheit. Aber man muß ja nicht alles glauben...
-
loks schrieb:
Sehr gewagte Aussage. ZUm einen ist es relativ schwer zu erkennen das in allen Fällen ein "return FALSE" passiert ausser wenn die while-schleife komplett durchlaufen wurde. Dazu muss man erstmal raussuchen an welchen Stellen retVal überall geändert wird und dann verstehen das bei sämtlichen gotos der Wert immer FALSE ist.
ach mist, das ist mir wirklich nicht aufgefallen. ausserdem fehlt beim label noch eine 'nachbehandlung', ohne sind die gotos ja auch leicht gegen returns zu ersetzen. soll ich noch was raussuchen oder lieber nicht? der microsoft-code ist die reinste goto-hölle, ich hab' nach kurzer zeit die suche abgebrochen, aber schon da stand es:
---- goto Matches (5381 in 470 files) ----
^^es wurde übrigens nur nach dem schlüsselwort 'goto' gesucht, nicht in kommentaren und so.
-
;fricky schrieb:
BOOL IE401IndexFile::EnumHashValues() { BOOL retVal = FALSE; if( m_pBuf == NULL) goto doneEnumHashValues; HASH_FILEMAP_ENTRY* pTable; HASH_ITEM* pItem; // pTable is located by an offset which is located at dwHashTableOffset pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); // The first location in the table follows immediately after the HASH_FILEMAP_ENTRY pTable pItem = (HASH_ITEM*)(pTable + 1); if (pTable->dwSig != SIG_HASH) goto doneEnumHashValues; do // Scan the list of tables. { // Scan the current table. for (; (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) { // call virtual entry handler if( HandleHashElement( pItem) == FALSE) goto doneEnumHashValues; } // Follow the link to the next table. if (!pTable->dwNext) { pTable = NULL; } else { // Validate the table signature and sequence number. DWORD nBlock; nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->dwSig != SIG_HASH || pTable->nBlock != nBlock + 1) goto doneEnumHashValues; // Set pointer to first location in table. pItem = (HASH_ITEM*) (pTable + 1); } } while (pTable); retVal = TRUE; doneEnumHashValues: return retVal; }Verdammt, der Quellcode kommt mir so bekannt vor!
Woher weißt du, welches das Projekt war, in dem ich meine goto-Phobie erworben habe?!
Da kommen schon die ersten gelben Flecken.... Und Atemnot....
Ruft bitte einer nen Krankenwagen!!!
Ste04tya<vdfvbl .cxdfg b.........
Hier spricht der Sanitäter. Der Patient ist mit dem Kopf auf die Tastatur geknallt. Bitte diesen Post nicht beachten!
-
"Ich sortiere mal ein Wenig um, dabei wird's leider die Kommentare zerhauen." "Zuerstmal sind diese ganzen gotos sinnlos. Und retVal auch." BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; HASH_FILEMAP_ENTRY* pTable; HASH_ITEM* pItem; // pTable is located by an offset which is located at dwHashTableOffset pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); // The first location in the table follows immediately after the HASH_FILEMAP_ENTRY pTable pItem = (HASH_ITEM*)(pTable + 1); if (pTable->dwSig != SIG_HASH) return FALSE; do // Scan the list of tables. { // Scan the current table. for (; (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) { // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; } // Follow the link to the next table. if (!pTable->dwNext) { pTable = NULL; } else { // Validate the table signature and sequence number. DWORD nBlock; nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->dwSig != SIG_HASH || pTable->nBlock != nBlock + 1) return FALSE; // Set pointer to first location in table. pItem = (HASH_ITEM*) (pTable + 1); } } while (pTable); return TRUE; }"Ein paar Definitionen später machen." BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; // pTable is located by an offset which is located at dwHashTableOffset HASH_FILEMAP_ENTRY* pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); if (pTable->dwSig != SIG_HASH) return FALSE; // The first location in the table follows immediately after the HASH_FILEMAP_ENTRY pTable HASH_ITEM* pItem = (HASH_ITEM*)(pTable + 1); do // Scan the list of tables. { // Scan the current table. for (; (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) { // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; } // Follow the link to the next table. if (!pTable->dwNext) { pTable = NULL; } else { // Validate the table signature and sequence number. DWORD nBlock; nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->dwSig != SIG_HASH || pTable->nBlock != nBlock + 1) return FALSE; // Set pointer to first location in table. pItem = (HASH_ITEM*) (pTable + 1); } } while (pTable); return TRUE; }"Verfummeln der Schleifenvariablen statt break sehe ich da." BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; // pTable is located by an offset which is located at dwHashTableOffset HASH_FILEMAP_ENTRY* pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); if (pTable->dwSig != SIG_HASH) return FALSE; // The first location in the table follows immediately after the HASH_FILEMAP_ENTRY pTable HASH_ITEM* pItem = (HASH_ITEM*)(pTable + 1); do // Scan the list of tables. { // Scan the current table. for (; (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) { // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; } // Follow the link to the next table. if (!pTable->dwNext) break; // Validate the table signature and sequence number. DWORD nBlock; nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->dwSig != SIG_HASH || pTable->nBlock != nBlock + 1) return FALSE; // Set pointer to first location in table. pItem = (HASH_ITEM*) (pTable + 1); } while (pTable); return TRUE; }"Doppelter Code." BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; // pTable is located by an offset which is located at dwHashTableOffset HASH_FILEMAP_ENTRY* pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); if (pTable->dwSig != SIG_HASH) return FALSE; // The first location in the table follows immediately after the HASH_FILEMAP_ENTRY pTable HASH_ITEM* pItem; do // Scan the list of tables. { pItem = (HASH_ITEM*) (pTable + 1); // Scan the current table. for (; (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) { // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; } // Follow the link to the next table. if (!pTable->dwNext) break; // Validate the table signature and sequence number. DWORD nBlock; nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->dwSig != SIG_HASH || pTable->nBlock != nBlock + 1) return FALSE; } while (pTable); return TRUE; }"Oh, dann kann pItem auch lokaler werden." BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; // pTable is located by an offset which is located at dwHashTableOffset HASH_FILEMAP_ENTRY* pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); if (pTable->dwSig != SIG_HASH) return FALSE; do // Scan the list of tables. { // Scan the current table. for (HASH_ITEM* pItem=(HASH_ITEM*) (pTable + 1); (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) { // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; } // Follow the link to the next table. if (!pTable->dwNext) break; // Validate the table signature and sequence number. DWORD nBlock; nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->dwSig != SIG_HASH || pTable->nBlock != nBlock + 1) return FALSE; } while (pTable); return TRUE; }"Schon möchte die Schleife da raus." BOOL ScanTheCurrentTable(HASH_FILEMAP_ENTRY* pTable) { for (HASH_ITEM* pItem=(HASH_ITEM*) (pTable + 1); (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; return TRUE; } BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; // pTable is located by an offset which is located at dwHashTableOffset HASH_FILEMAP_ENTRY* pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); if (pTable->dwSig != SIG_HASH) return FALSE; do // Scan the list of tables. { if(!ScanTheCurrentTable(pTable)) return FALSE; // Follow the link to the next table. if (!pTable->dwNext) break; // Validate the table signature and sequence number. DWORD nBlock; nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->dwSig != SIG_HASH || pTable->nBlock != nBlock + 1) return FALSE; } while (pTable); return TRUE; }"Das break war auch ein return." BOOL ScanTheCurrentTable(HASH_FILEMAP_ENTRY* pTable) { for (HASH_ITEM* pItem=(HASH_ITEM*) (pTable + 1); (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; return TRUE; } BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; // pTable is located by an offset which is located at dwHashTableOffset HASH_FILEMAP_ENTRY* pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); if (pTable->dwSig != SIG_HASH) return FALSE; do // Scan the list of tables. { if(!ScanTheCurrentTable(pTable)) return FALSE; // Follow the link to the next table. if (!pTable->dwNext) return FALSE; // Validate the table signature and sequence number. DWORD nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->dwSig != SIG_HASH || pTable->nBlock != nBlock + 1) return FALSE; } while (pTable); return TRUE; }"Geht eigentlich alles locker von der Hand." "Mal nur zum Spaß wirr weitersuchen." "pTable->dwSig != SIG_HASH mal abtrennen." BOOL ScanTheCurrentTable(HASH_FILEMAP_ENTRY* pTable) { for (HASH_ITEM* pItem=(HASH_ITEM*) (pTable + 1); (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; return TRUE; } BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; // pTable is located by an offset which is located at dwHashTableOffset HASH_FILEMAP_ENTRY* pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); if (pTable->dwSig != SIG_HASH) return FALSE; do // Scan the list of tables. { if(!ScanTheCurrentTable(pTable)) return FALSE; // Follow the link to the next table. if (!pTable->dwNext) return FALSE; // Validate the table signature and sequence number. DWORD nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->nBlock != nBlock + 1) return FALSE; if (pTable->dwSig != SIG_HASH) return FALSE; } while (pTable); return TRUE; }"Und diese Duplikation auch wegmachen." BOOL ScanTheCurrentTable(HASH_FILEMAP_ENTRY* pTable) { for (HASH_ITEM* pItem=(HASH_ITEM*) (pTable + 1); (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; return TRUE; } BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; // pTable is located by an offset which is located at dwHashTableOffset HASH_FILEMAP_ENTRY* pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); do // Scan the list of tables. { if (pTable->dwSig != SIG_HASH) return FALSE; if(!ScanTheCurrentTable(pTable)) return FALSE; // Follow the link to the next table. if (!pTable->dwNext) return FALSE; // Validate the table signature and sequence number. DWORD nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->nBlock != nBlock + 1) return FALSE; } while (pTable); return TRUE; }"Bachte, daß das bloß Codeschubserei ist, die klappt, ohne," "daß ich überhaupt weiß, was der Code macht." "Deswegen auch der schlechte Name der ausgelagerten Schleife." "Den hab ich vom Kommentar geklaut." "Vielleicht ist sowas gemeint, keine Ahnung." BOOL IsTableFull(HASH_FILEMAP_ENTRY* pTable) { for (HASH_ITEM* pItem=(HASH_ITEM*) (pTable + 1); (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; return TRUE; } BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; // pTable is located by an offset which is located at dwHashTableOffset HASH_FILEMAP_ENTRY* pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); do // Scan the list of tables. { if (pTable->dwSig != SIG_HASH) return FALSE; if(!IsTableFull(pTable)) return FALSE; // Follow the link to the next table. if (!pTable->dwNext) return FALSE; // Validate the table signature and sequence number. DWORD nBlock = pTable->nBlock; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); if (pTable->nBlock != nBlock + 1) return FALSE; } while (pTable); return TRUE; }"Vielleicht man nBlock wegmachen." "Nebenbei fange ich langsam an, den Code zu verstehen und kann versuchen," "die Kommentare zu heilen." BOOL IsTableFull(HASH_FILEMAP_ENTRY* pTable) { for (HASH_ITEM* pItem=(HASH_ITEM*) (pTable + 1); (LPBYTE)pItem < (LPBYTE)pTable + BYTES_PER_TABLE; pItem++) // call virtual entry handler if ( HandleHashElement( pItem) == FALSE) return FALSE; return TRUE; } BOOL IE401IndexFile::EnumHashValues() { if ( m_pBuf == NULL) return FALSE; // pTable is located by an offset which is located at dwHashTableOffset HASH_FILEMAP_ENTRY* pTable = (HASH_FILEMAP_ENTRY*)(m_pBuf + (((_MEMMAP_HEADER_SMALL*)m_pBuf)->dwHashTableOffset)); do // Scan the list of tables. { // Validate the table signature if (pTable->dwSig != SIG_HASH) return FALSE; if(!IsTableFull(pTable)) return FALSE; // Move to next table HASH_FILEMAP_ENTRY* oldTable = pTable; if (!pTable->dwNext) return FALSE; pTable = (HASH_FILEMAP_ENTRY*) (m_pBuf + pTable->dwNext); // Validate the table sequence number. if (pTable->nBlock != oldTable->nBlock + 1) return FALSE; } while (pTable); return TRUE; }Du siehst, es ist nicht nur keine, aber auch nicht die geringste
Notwendigkeit für ein goto, darüberhinaus war der Code durchaus
ein wenig reparaturbedürftig. Außerdem fühlte sich der Code so
an, als sei es eigentlich C, das in eine Klasse gepresst wurde.
-
;fricky schrieb:
ach mist, das ist mir wirklich nicht aufgefallen. ausserdem fehlt beim label noch eine 'nachbehandlung', ohne sind die gotos ja auch leicht gegen returns zu ersetzen. soll ich noch was raussuchen oder lieber nicht?
Der Code ist für mich nicht leicht verständlich. Ich weiß ja nicht, was EnumHashValues tut. Nur eins weiß ich, die HashValues werden nicht Enumeriert. Wenn alle Bezeichner unverständlich sind und die Datenstrukturen auch unklar, kann ich evtl nichts umbasteln. Aber die Wahrscheinlichkeit ist sehr groß, daß C++-Code, der das gleiche tut, gar keinen Bedarf für ein goto hätte.
Ich behaupte nicht, daß goto immer schlecht wäre. Die Gründe, weshalb goto hier und da wegfliegt, sind immer andere, mal weil die Compiler stärker sind und inline können, mal RAII, mal RVO, mal dies mal das mal solches, mal nichtmal ein Sprachmittel, sondern neu entdeckter Stil, aber goto ist nie weit weg, und oft mein erster Ansatz, was zu leicht geschriebenen schwer zu lesendenm Code führt, den ich dann umbaue in erst mit Umbau geschriebenen leicht zu lesenden Code, zusammen ist das so unglaublich vielschichtig, daß man keine Beweisführung versuchen sollte, daß goto immer duch was besseres ersetzt werden kann. Ich kann nur sagen, daß ich goto anscheinend nicht brauche. Deswegen schaue ich immer, wenn ich goto geschrieben habe, ob ich nicht einen Denkfehler gemacht habe, der mich zum goto zwang, und ob nicht eine viel einfachere Ansicht des Problems zu einfacherem Code ohne goto führt. Irgendwie ist es immer so, seit vielen Jahren.
