È un odore di codice di pausa?

Avere un break fuori da un ciclo non è diverso dal fatto che il loop viene rifattorizzato con una funzione propria e un'istruzione return in una clausola di guardia.

while(condition) {
  if(test) { break; }
  doStuff;
}

vs

doMuchStuff();

function doMuchStuff() {
  while(condition) {
    if(test) { return; }
    doStuff;
  }
}

Questi sono effettivamente gli stessi.

Le singole parole chiave non sono odori di codice. Sono strumenti per il controllo del flusso. Molti di questi sono variazioni su goto con un altro livello di sicurezza per evitare l'incubo del codice spaghetti.

Nel giudicare se un particolare bit di codice è problematico, è necessario considerare come funziona e se è appropriato per i costrutti del linguaggio. La parola chiave da sola non è sufficiente per stabilire un odore di codice. Potrebbe essere un soffio in quanto può essere usato impropriamente, ma andare in caccia alle streghe perché si vede un switch o break o qualche altro costrutto di controllo del flusso è controproducente e porta a guide di stile che impediscono alle persone di scrivere in modo comprensibile, diretto codice.

@MichaelT ha chiesto come riscrivere il codice di esempio C # per goto senza usare goto. Ecco il loro codice:

using System;
class Test
{
   static void Main(string[] args) {
      string[,] table = {
         {"Red", "Blue", "Green"},
         {"Monday", "Wednesday", "Friday"}
      };
      foreach (string str in args) {
         int row, colm;
         for (row = 0; row <= 1; ++row)
            for (colm = 0; colm <= 2; ++colm)
               if (str == table[row,colm])
                   goto done;
         Console.WriteLine("{0} not found", str);
         continue;
   done:
         Console.WriteLine("Found {0} at [{1}][{2}]", str, row, colm);
      }
   }
}

La prima cosa che si nota è che, come molti esempi / codice demo, stampa il risultato. Qualsiasi codice demo che stampa il risultato è B.S. Dal momento che quasi nessuna funzione reale stampa il risultato. Dovrebbe restituire un risultato, modificare un oggetto, accodare a un flusso, ecc ...

(Segmento secondario ...) In questo caso, non è un grosso problema, ma prova il codice Node.js "demo" dove ti trovi in profondità in un terzo callback asincrono nidificato, chiedendoti come diavolo si sta per abituare a quel risultato, e la demo ignora il problema stampando qualcosa. Grrr. :-(

Ma questo ti fa pensare. Quel codice sta stampando e sta facendo più di una cosa. Ora, sono meno fanatico di molti rispetto al Principio di Responsabilità Unica, ma, se sei così bloccato nella melma che un goto sembra buono, stai violando SRP .

Accidenti, cosa succede se mi rifatto e scrivo una funzione generica per cercare in un array 2D? Ecco la riscrittura iniziale: Nota - Non sono un programmatore C #, quindi questo è in Java.

public class Test {

    public static void Main(String[] args) {
          String[][] table = {
             {"Red", "Blue", "Green"},
             {"Monday", "Wednesday", "Friday"}
          };
          for (String str : args) {
             int[] found = indexOf2D(table, str);
             if (found == null)
                 System.out.println(str + " not found");
             else
                 System.out.println(str + " found at " + found);
       }
    }


    public static int[] indexOf2D(Object[][] array, Object grail) {
        for (int r = 0; r<array.length; r++) {
            Object[] row = array[r];
            for (int c = 0; c<row.length; c++)
                if (grail.equals(row[c]))
                    return new int[] {r,c};
        }
        return null;
    }
}

Ora, in un programma reale, rifaresti di nuovo, scrivendo una funzione di "ricerca in una matrice 1D" generica. E tu verificherai gli input nulli. Dovresti discutere di restituire null rispetto a un "Oggetto Null" rispetto a un Opzionale. Si finisce con il codice che è un'abilità volte più versatile, più robusto ed è chiaramente preso in considerazione.

E il codice non usa un goto folle. Come detto prima, Se il tuo codice è così complesso che è necessario utilizzare un goto, il tuo codice è troppo complesso .

"Oh, ma hai aggiunto altre due chiamate di metodo e una creazione di oggetti, che rallenterà le cose". Bah. Se questo viene chiamato solo occasionalmente, i pochi nanosecondi aggiuntivi non contano. E, se questo codice è critico nel tempo, forse dovresti riconsiderare l'uso di una ricerca lineare O (N) attraverso la tua struttura dati 2D. : -)

Un commento sul motivo per cui interruzione / continua è "più maleodorante" di ritorno .

I Resi sono più facili da ragionare sul programmatore. Esci dal metodo, lo stack e tutte le variabili locali vanno via. In effetti, qualsiasi informazione di tipo "chiusura" per quel metodo scompare e non devi preoccuparti di ciò. (ehm, a meno che tu non abbia aperto risorse come i file che devono essere chiusi ...)

Con una interruzione o continua , vai avanti nel codice, ma c'è stato locale ancora da ricordare . Questo può essere difficile. Quali variabili locali sono ancora in ambito? Qual è il valore alla fine del ciclo? Il programmatore che legge il codice ha in gran parte la "chiusura" di quello stato nella testa.

Le chiusure sono utili e interessanti, ma in generale è meglio che il compilatore o l'interprete mantengano tutto quello stato, non la mia debole mente.

Sì, è un "odore di codice".

Questo richiama i giorni del codice spagettii, gotos, sosubs e rottura di loop per portare avanti una procedura quando erano state raggiunte diverse condizioni.

Queste pratiche sono state quasi universalmente sostituite con i metodi di stile OO, mentre, foreach e inhertiance. Pertanto, se hai ancora interruzioni nel codice dovresti chiederti se ne hai bisogno.

Ovviamente, però, "code smell" è solo un indicatore che qualcosa potrebbe essere sbagliato. Non è una regola difficile che non può mai essere infranta.

Inoltre dovremmo notare che una funzione che ritorna da più di un posto è anche un odore di codice.

es:

function doMuchStuff() {
  while(condition) {
    if(test) { return; }
    doStuff;
  }
}

dovrebbe essere

function doMuchStuff() {
  while(condition && !test) {
    doStuff;
  }
}

per evitare il tipo di incubo con cui puoi entrare

function doMuchStuff() {
  while(condition) {
    if(test) { return; }
    doStuff;
    if(test2) { return stuff; }
    doStuff2;
    if(test3) { return differentStuff; }
    doStuff3;
...etc
      }
    }