Decremento / Incremento della variabile di ciclo all'interno di loop. Questo codice puzza?

4

Devo leggere le righe da un file di testo in ordine sequenziale. Il file è un formato di testo personalizzato che contiene sezioni. Se alcune sezioni sono fuori servizio, vorrei cercare l'avvio della prossima sezione valida e continuare l'elaborazione.

Attualmente ho un codice simile a questo:

for (int currentLineIndex=0; currentLineIndex < lines.Count; currentLineIndex++ )
{
    //Process section here
    if( out_of_order_condition )
    {
        currentLineIndex--;//Stay on the same line in the next iteration because this line may be the start of a valid section.
       continue;
    }
}

Questo codice è olfattivo?

AGGIORNAMENTO: non l'ho menzionato prima, ma la causa principale di questo tipo di codice era una complicata commutazione (tipica durante l'analisi).

Mi sono liberato della variabile di incremento / decremento usando la frase "goto case".

La struttura ora appare così:

switch(state)
{
   case State.BOF:
       {
           //Process BOF case
       }
   case State.SeenHeader:
       {
           if( out_of_order_condition )
           {
               state = State.BOF;    //Reset the state to some respectable one

               //currentLineIndex--; Removed
               //continue;           Removed

               goto case State.BOF;//Handle this in this iteration itself. 
           }
       }
}
    
posta Faredoon 18.12.2012 - 03:40
fonte

4 risposte

6

Bene, dal momento che un odore di codice è qualcosa che ti fa dare una seconda occhiata, che tu stesso stai facendo, direi che sicuramente lo qualifica. Tuttavia, gli odori di codice non richiedono automaticamente la rimozione, solo un aspetto difficile per assicurarsi che sia davvero il modo migliore per risolvere il problema.

In questo caso particolare, il motivo per cui non vedi spesso un codice del genere è che il loop non può mai terminare in determinate condizioni di input. Stai anche commistendo due diverse responsabilità nel ciclo: la sezione di rilevamento inizia ed elabora una sezione. Vorrei provare ad avere un loop che rileva solo i limiti della sezione, e una volta che l'intera sezione è conosciuta, passa il contenuto della sezione ad un'altra funzione per l'elaborazione.

Probabilmente è anche un segno che il tuo formato è abbastanza complesso da far sì che un parser homegrown abbia difficoltà a catturare tutte le condizioni al contorno. Potresti voler esaminare un parser completo come bisonte o antlr.

    
risposta data 18.12.2012 - 05:00
fonte
8

Penso che sia un odore di codice. L'idioma in un ciclo for consiste nell'incrementare / decrementare in modo coerente. Le persone che leggono il tuo codice si aspettano che l'espressione di incremento (currentLineIndex ++) sia la logica di controllo.

Penso che sarebbe meglio fare una delle seguenti azioni:

  1. trasforma questo codice in un ciclo while. L'idioma per i cicli while varia di molto, rendendo gli sviluppatori più propensi a leggere il tuo codice.
  2. aggiungi un ciclo while annidato per gestire il caso in cui rimani su una riga. In questo modo il ciclo esterno conserva l'idioma e rendi più ovvio che il ciclo interno continua fino a quando non viene più soddisfatta la condizione out_of_order_.
risposta data 18.12.2012 - 05:02
fonte
0

No. O almeno I non la penso così. È un'indicazione che l'iterazione è complicata, ma non è necessariamente un segno di un problema di progettazione fondamentale ... o di una cattiva pratica di codifica.

Tuttavia, potresti voler guardare il codice e pensare se esiste un modo più leggibile per farlo. E se mantieni il codice così com'è, puoi:

  • aggiungi un commento all'inizio del ciclo per dire che la variabile loop è cambiata, o

  • cambia questo in while loop.

D'altra parte "l'odore del codice" è molto soggettivo ...

    
risposta data 18.12.2012 - 04:04
fonte
0

Dipende. Per essere più sicuri sullo scenario multi-thread, utilizza Interlocked. * API piuttosto rispetto all'utilizzo di operatori semplici.

    
risposta data 20.12.2012 - 04:29
fonte

Leggi altre domande sui tag