do{}while(false); -- ein besseres goto?
-
;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.
-
volkard schrieb:
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.
kenne ich, ich hab z.b. was goto angeht sowas wie 'design patterns' im kopf. wenn ich mal goto verwenden will, dann dann läuft bei mir etwa dieses ab:
1. bekanntes, unproblematisches goto-muster (nested loops, cleanup, usw.): goto kommt rein, es wird nicht weiter drüber nachgedacht, hat noch nie gezickt oder zu verwirrung geführt.
2. leichte abweichung von (1): alarmglocke bimmelt, suboptimale programmstruktur wird angenommen und versucht des goto zu umgehen. wenn der code wider erwarten mit goto einfacher bleibt, wird goto eingesetzt.
3. starke abweichung von (1): ähnlich (2), resultiert aber so gut wie nie in einem goto, weil die funktion meistens schrottig programmiert war und komplett umgestellt werden muss.
-
Ich habe festgestellt, dass echte Programmierer mit Sprüngen in Assembler-Code klar kommen. Das heißt ohne, dass sie Ausschläge, Haarausfall, oder Sonstiges bekommen.
Dementsprechend kommen Programmierer auch mit gotos in Hochsprachen klar. Fricky's Snippet ist doch ein Bilderbuchbeispiel, denn jeder echte Programmierer kann den Code ohne Probleme lesen. Und das zählt, denn in der Realität werdet ihr meist keinen Bilderbuchcode zu Gesicht bekommen. Wer also nicht in der Lage ist auch sub-optimalen Code lesen zu können, der taugt als Programmierer einfach nichts (keine Sorge gibt genug Sparten wo es auch ohne geht: der Java, PHP und VB Markt ist groß).
Wichtig ist doch nur, dass eine Funktion keine zwei-stelligen Verschachtelungstiefen enthält. Selbst wenn eine Funktion mal 200 Zeilen lang ist so heißt das noch lange nicht, dass man nicht ohne größere Probleme nachvollziehen kann was sie tut.Am Ende ist es das was für mich zählt, nämlich, dass ich den Code ohne größere Hürden lesen kann. Und wenn ein Kollege dann mal eine 200 Zeilen-Funktion mit einem goto irgendwo hat, dann verteufel ich ihn deshalb nicht. Ich verwende zwar kein goto außerhalb von Cleanup-Code, aber beim schnellen prototyping einer Funktion wird die bei mir auch mal länglich (wobei tief-verschachtelte Schleifen meist Funktionalität bedeuten, was ich direkt auslagere, d.h. meine länglichen Funktionen sind meist nur viel boiler-plate Code; sprich fehlerüberprüfung und Aufrufen anderer Funktionen) und ich will oder kann sie nicht sofort zerkleinern, da noch gar nicht absehbar ist ob sie so bleibt. Und manchmal vergisst man es dann auch ganz einfach ...
Mir ist auch klar, dass es in dem Projekt, aus welchem fricky's Beispiel ist, der Code überall so oder so ähnlich aussehen wird (und nicht meinem Stil entspricht), aber Leseprobleme werde ich wohl kaum haben (Funktion passt auf einen Bildschirm und hat Kommentare - was will ich mehr!? <- rhetorische Frage!).
-
Echter Progger schrieb:
Dementsprechend kommen Programmierer auch mit gotos in Hochsprachen klar. Fricky's Snippet ist doch ein Bilderbuchbeispiel, denn jeder echte Programmierer kann den Code ohne Probleme lesen. Und das zählt, denn in der Realität werdet ihr meist keinen Bilderbuchcode zu Gesicht bekommen. Wer also nicht in der Lage ist auch sub-optimalen Code lesen zu können, der taugt als Programmierer einfach nichts (keine Sorge gibt genug Sparten wo es auch ohne geht: der Java, PHP und VB Markt ist groß).
Wichtig ist doch nur, dass eine Funktion keine zwei-stelligen Verschachtelungstiefen enthält. Selbst wenn eine Funktion mal 200 Zeilen lang ist so heißt das noch lange nicht, dass man nicht ohne größere Probleme nachvollziehen kann was sie tut.Und wer nicht in der Lage ist, den suboptimalen Code in etwas besseren Code zu ändern, wenn er eh grade dran sitzt, der taugt genausowenig wie derjenige, der den suboptimalen Code stehengelassen hat als er ihn fabriziert hat. Dazu zählen auch 200-Zeilen-Funktionen. "Ich kann Spaghetticode lesen, also darf ich auch Spaghetticode schreiben" ist genausowenig ein Argument wie "Ich hab zwar scheiß geschrieben, aber der Code war schon so scheiße, da macht das auch nichts mehr".
sprich fehlerüberprüfung und Aufrufen anderer Funktionen) und ich will oder kann sie nicht sofort zerkleinern, da noch gar nicht absehbar ist ob sie so bleibt. Und manchmal vergisst man es dann auch ganz einfach ...
Streich das "ich kann sie nicht zerkleinern" - du kannst schon, nur willst du nicht. Und wenn du wirklich ein echter Programmierer wärst, dann wüsstest du auch dass man nicht manchmal sondern quasi immer vergisst, es hinterher richtig zu machen, und dass es sehr wohl absehbar ist, dass die Funktion mit ziemlicher Sicherheit so bleibt wie sie gerade ist. Jedes Provisorium ist dauerhaft in dieser Branche.
-
Echter Progger schrieb:
Ich habe festgestellt, dass echte Programmierer mit Sprüngen in Assembler-Code klar kommen. Das heißt ohne, dass sie Ausschläge, Haarausfall, oder Sonstiges bekommen. Dementsprechend kommen Programmierer auch mit gotos in Hochsprachen klar...
Manche Programmierer versuchen, im Assembler-Code auch so wenig wie möglich Sprünge einzubauen. Weil beim Ausführen eines Sprungbefehls in der CPU die "Pipeline" (was immer man unter der Pipeline versteht) "entleert" und wieder "befüllt" werden muss, was vertane CPU Takte bedeutet.
-
das gilt aber nur unter bestimmten und ungünstigen Umständen, zB wenn die branch-prediction eine bedingte Verzweigung falsch vorhersagt (=> pipeline- und cache-flush). Die branch-prediction units moderner RISC-CPUs können Sprünge schon ziemlich gut vorhersagen und damit pipeline hazards usw vermeiden.
Oder natürlich, wenn die CPU gar keine branch-prediction hat - dann wird die CPU aber eventuell (wahrscheinlich) auch keine pipeline haben, und dann ist es wieder mehr oder minder egal, ob ein Sprung stattfindet oder nicht.
