Refactoring weg markierte Schlaufen

stimmen
18

Nachdem ich überzeugt war , dass markierte Brüche / weiterhin gibt insgesamt „Nono“ über hier , ich brauche Hilfe das Etikett aus meinem Code zu entfernen.

Ich habe eine quadratische Matrix und einen Vektor, der die gleiche Länge aufweist. Der Vektor hat bereits einige Werte in Abhängigkeit es eine auf den Werten in der Matrix der Vektor in der Schleife geändert wird.

Ich hoffe, das Code-Fragment im Grunde verständlich ist ...

vectorLoop:
for( int idx = 0; idx < vectorLength; idx++) {
    if( conditionAtVectorPosition( v, idx ) ) continue vectorLoop;

    matrixLoop:
    for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
        if( anotherConditionAtVector( v, rowIdx ) ) continue matrixLoop;
        if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) continue vectorLoop;
    }
    setValueInVector( v, idx );
}     

Bitte überzeugen Sie mir, dass es eine besser lesbare / bessere Version ohne die Etiketten.

Veröffentlicht am 19/08/2008 um 08:42
quelle vom benutzer
In anderen Sprachen...                            


12 antworten

stimmen
33

Mit Blick auf den bisher vorgestellten Lösungen:

  • Sie sehen alle weniger lesbar als das Original, dass sie beinhalten die Ausgaben mehr Code auf dem Mechanismus des Codes und nicht auf den Algorithmus selbst

  • Einige von ihnen sind gebrochen oder waren, bevor sie bearbeitet wurden. Die meisten vernichtend ist die Tatsache, dass die Menschen sehr schwer zu denken haben, wie Sie den Code ohne Etiketten zu schreiben und nichts kaputt machen.

  • Einige sind mit einer Leistungseinbuße von dem gleichen Test zweimal ausgeführt wird, was nicht immer trivial sein kann. Die Alternative dazu ist die Speicherung und runde booleans vorbei, die hässlich werden.

  • Refactoring den relevanten Teil des Codes in eine Methode ist effektiv ein no-op: es umlagert, wie der Code in der Datei angelegt, hat aber keinen Einfluss darauf, wie sie ausgeführt wird.

All das macht mich glauben, dass, zumindest im Fall dieser Frage formuliert, ist das Label die richtige Lösung und muss nicht Refactoring entfernt werden. Sicherlich gibt es Fälle, in denen Etiketten falsch verwendet werden und sollte Refactoring entfernt werden. Ich glaube einfach nicht, es als eine unzerbrechliche Regel behandelt werden soll.

Beantwortet am 19/08/2008 um 11:14
quelle vom benutzer

stimmen
1

Ich denke, dass markierte Schleifen so selten sind, dass Sie, was Verfahren zur Markierung arbeitet für Sie auswählen können - was Sie haben es macht Ihre Absichten mit der vollkommen klar bleibt.


Nachdem er die Ladung der Schleifen in der ursprünglichen Frage vorschlägt Refactoring und jetzt den Code in Frage zu sehen, glaube ich Ihnen eine sehr lesbare Schleife dort haben.

Was ich hatte mir vorgestellt, war ein ganz anderes Stück Code - das eigentliche Beispiel das Aufstellen, ich kann sehen, es ist viel sauberer als ich gedacht hatte.

Ich entschuldige mich für das Missverständnis.

Beantwortet am 20/08/2008 um 05:26
quelle vom benutzer

stimmen
1

Diese Frage war nicht über den Algorithmus zu optimieren - aber trotzdem danke ;-)

Damals habe ich es geschrieben habe, als ich die weiterhin als eine lesbare Lösung bezeichnet.

Ich fragte SO eine Frage über die Konvention (mit dem Etikett in allen Kappen oder nicht) für Etiketten in Java.

Grundsätzlich ist jede Antwort hat mir gesagt, „benutzen sie nicht - es gibt immer einen besseren Weg, Umgestalten!“. Also gab ich diese Frage für eine besser lesbare (und damit besser?) Zu fragen Lösung.

Bis jetzt bin ich nicht ganz überzeugt von den Alternativen bisher vorgestellt.

Bitte verstehen Sie mich nicht falsch. Labels sind böse die meiste Zeit.

Aber in meinem Fall sind die bedingten Tests ziemlich einfach, und der Algorithmus von einem mathematischen Papier entnommen wird und daher sehr wahrscheinlich nicht in naher Zukunft zu ändern. So habe ziehe ich alle relevanten Teile auf einmal sichtbar anstatt zu einem anderen Methode mit dem Namen etwas wie checkMatrixAtRow (x) blättern.

Vor allem bei komplexeren mathematischen Algorithmen, finde ich es ziemlich hart „gut“ funktions Namen zu finden - aber ich denke, das ist noch eine andere Frage

Beantwortet am 19/08/2008 um 17:28
quelle vom benutzer

stimmen
1
Einige sind mit einer Leistungseinbuße von dem gleichen Test zweimal ausgeführt wird, was nicht immer trivial sein kann. Die Alternative dazu ist die Speicherung und runde booleans vorbei, die hässlich werden.
Die Leistungseinbuße ist gering. Ich stimme jedoch, dass ein Test zweimal ausgeführt ist keine schöne Lösung.

Ich glaube, die Frage war, wie die Etiketten zu entfernen, nicht, wie man den Algorithmus zu optimieren. Es schien mir, dass das ursprüngliche Plakat war nicht bewusst, wie verwenden ‚Weiter‘ und ‚Bruch‘ Schlüsselwörter ohne Etiketten, aber natürlich können meine Annahmen falsch sein.

Wenn es um Leistung geht, wird die Post keine Informationen über die Umsetzung der anderen Funktionen geben, so dass für alle weiß, dass ich sie könnte genauso gut wie die aus einfachen Berechnungen die Ergebnisse per FTP werden Download vom Compiler inlined.

Davon abgesehen, zweimal den gleichen Test machen, ist nicht optimal in der Theorie.

EDIT: Auf einem zweiten Gedanken, das Beispiel ist eigentlich nicht eine schreckliche Verwendung von Etiketten. Ich bin damit einverstanden , dass „goto ist ein no-no“ , aber nicht wegen des Code wie folgt. Die Verwendung von Etiketten hier nicht tatsächlich Einfluss auf die Lesbarkeit des Codes in signifikanter Weise. Natürlich sind sie nicht erforderlich und leicht verzichtet werden kann, aber nicht mit ihnen , nur weil „Etiketten mit schlecht ist“ in diesem Fall kein gutes Argument. Immerhin macht die Etiketten zu entfernen den Code nicht viel einfacher zu lesen, haben wie andere bereits kommentiert.

Beantwortet am 19/08/2008 um 13:15
quelle vom benutzer

stimmen
1

@ Nicolas

Einige von ihnen sind gebrochen oder waren, bevor sie bearbeitet wurden. Die meisten vernichtend ist die Tatsache, dass die Menschen sehr schwer zu denken haben, wie Sie den Code ohne Etiketten zu schreiben und nichts kaputt machen.

Ich habe einen anderen Blickwinkel: Einige von ihnen sind gebrochen, weil es schwierig ist, das Verhalten des ursprünglichen Algorithmus, um herauszufinden.

Ich weiß, dass es ist subjektiv, aber ich habe keine Probleme den ursprünglichen Algorithmus zu lesen. Es ist kürzer und klarer als der vorgeschlagenen Ersatz.

Was alle Refactorings in diesem Thread tun ist, um das Verhalten eines Etiketts mit anderen Sprachfunktionen emulieren - als ob Sie den Code in eine Sprache wurden die Portierung, die nicht mit Zetteln hat.

Beantwortet am 19/08/2008 um 12:28
quelle vom benutzer

stimmen
1

Von Ihrem Code zu lesen.

  • Ich habe bemerkt, Sie die ungültigen Vektorpositionen bei conditionAtVectorPosition Eliminieren Sie dann die ungültigen Zeilen bei anotherConditionAtVector entfernen.
  • Es scheint, dass Reihen bei anotherConditionAtVector Überprüfung da redundant ist, was auch immer der Wert von idx ist, anotherConditionAtVector hängt nur von dem Zeilenindex (unter der Annahme anotherConditionAtVector hat keine Nebenwirkungen).

So können Sie dies tun:

  • Holen Sie sich die gültigen Positionen zuerst mit conditionAtVectorPosition (das sind die gültigen Spalten).
  • Dann erhalten die gültigen Zeilen anotherConditionAtVector verwenden.
  • Schließlich verwenden conditionAtMatrixRowCol die gültigen Spalten und Zeilen verwenden.

Ich hoffe das hilft.

Beantwortet am 19/08/2008 um 10:09
quelle vom benutzer

stimmen
1

@Patrick Sie gehen davon aus setValueInVector (v, idx) gefordert wird; am Ende der zweiten Schleife ist OK. Wenn der Code identisch zu sein ist, logisch, muss es neu geschrieben werden wie folgt somethng:

for (int idx = 0; idx 
Beantwortet am 19/08/2008 um 08:56
quelle vom benutzer

stimmen
1

Leicht, guter Mann.

for( int idx = 0; idx < vectorLength; idx++) {
  if( conditionAtVectorPosition( v, idx ) ) continue;

  for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
    if( anotherConditionAtVector( v, rowIdx ) ) continue;
    if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) break;
  }
  if( !conditionAtMatrixRowCol( m, rowIdx, idx ) )
    setValueInVector( v, idx );
}

EDIT: Ganz richtig Sie sind Anders. Ich habe meine Lösung bearbeitet als auch das zu berücksichtigen.

Beantwortet am 19/08/2008 um 08:45
quelle vom benutzer

stimmen
0

@ Sadie :

Sie sehen alle weniger lesbar als das Original, dass sie beinhalten die Ausgaben mehr Code auf dem Mechanismus des Codes und nicht auf den Algorithmus selbst

Externalisierung der zweiten Schleife außerhalb des Algorithmus ist nicht notwendigerweise weniger lesbar. Wenn die Methode Name gut gewählt wird, kann es die Lesbarkeit verbessern.

Einige von ihnen sind gebrochen oder waren, bevor sie bearbeitet wurden. Die meisten vernichtend ist die Tatsache, dass die Menschen sehr schwer zu denken haben, wie Sie den Code ohne Etiketten zu schreiben und nichts kaputt machen.

Ich habe einen anderen Blickwinkel: Einige von ihnen sind gebrochen, weil es schwierig ist, das Verhalten des ursprünglichen Algorithmus, um herauszufinden.

Einige sind mit einer Leistungseinbuße von dem gleichen Test zweimal ausgeführt wird, was nicht immer trivial sein kann. Die Alternative dazu ist die Speicherung und runde booleans vorbei, die hässlich werden.

Die Leistungseinbuße ist gering. Ich stimme jedoch, dass ein Test zweimal ausgeführt ist keine schöne Lösung.

Refactoring den relevanten Teil des Codes in eine Methode ist effektiv ein no-op: es umlagert, wie der Code in der Datei angelegt, hat aber keinen Einfluss darauf, wie sie ausgeführt wird.

Ich sehe nicht den Punkt. Ja, es ändert sich nicht das Verhalten, wie ... Refactoring?

Sicherlich gibt es Fälle, in denen Etiketten falsch verwendet werden und sollte Refactoring entfernt werden. Ich glaube einfach nicht, es als eine unzerbrechliche Regel behandelt werden soll.

Ich bin vollkommen einverstanden. Aber wie Sie haben darauf hingewiesen, haben einige von uns Schwierigkeiten während dieses Beispiel Refactoring. Auch wenn das anfängliche Beispiel lesbar ist, ist es schwer zu halten.

Beantwortet am 19/08/2008 um 12:01
quelle vom benutzer

stimmen
0

Ich bin nicht sicher, die erste weiter zu verstehen. Ich würde Gishu kopieren und schreiben so etwas wie (sorry wenn es einige Fehler):

for( int idx = 0; idx < vectorLength; idx++) {
    if( !conditionAtVectorPosition( v, idx ) && CheckedEntireMatrix(v))
        setValueInVector( v, idx );
}

inline bool CheckedEntireMatrix(Vector v) {
    for(rowIdx = 0; rowIdx < n; rowIdx++)
        if ( !anotherConditionAtVector(v,rowIdx) && conditionAtMatrixRowCol(m,rowIdx,idx) ) 
            return false;
    return true;
}
Beantwortet am 19/08/2008 um 09:50
quelle vom benutzer

stimmen
0

Gishu hat die richtige Idee:

for( int idx = 0; idx < vectorLength; idx++) {
    if (!conditionAtVectorPosition( v, idx ) 
        && checkedRow(v, idx))
         setValueInVector( v, idx );
}

private boolean checkedRow(Vector v, int idx) {
    for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
        if( anotherConditionAtVector( v, rowIdx ) ) continue;
        if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) return false;
    }  
    return true;
}
Beantwortet am 19/08/2008 um 09:46
quelle vom benutzer

stimmen
0

Geht das für dich? Ich die innere Schleife in ein Verfahren CheckedEntireMatrix extrahiert (Sie können es nennen besser als ich) - Auch mein Java ein bisschen rostig ist .. aber ich denke, es ist die Botschaft wird über

for( int idx = 0; idx < vectorLength; idx++) {
    if( conditionAtVectorPosition( v, idx ) 
    || !CheckedEntireMatrix(v)) continue;

    setValueInVector( v, idx );
}

private bool CheckedEntireMatrix(Vector v)
{
    for( rowIdx = 0; rowIdx < n; rowIdx++ ) {
        if( anotherConditionAtVector( v, rowIdx ) ) continue;
        if( conditionAtMatrixRowCol( m, rowIdx, idx ) ) return false;
    }   
    return true;
}
Beantwortet am 19/08/2008 um 08:56
quelle vom benutzer

Cookies help us deliver our services. By using our services, you agree to our use of cookies. Learn more