Omissione di indentazioni / parentesi in cicli nidificati per loop - cattiva pratica / stile? [chiuso]

1

Usando C # è pratica comune (credo) omettere il rientro e le parentesi con annidato usando affermazioni come questa:

using (var fileStream = new FileStream("filename"))
using (var streamReader = new StreamReader(fileStream))
using (var myCustomReader = new MyCustomReader(streamReader ))
{
    ///...
}

Recentemente ho rifatturato qualche codice che ha sofferto di nidificazione disordinata di if e for loops, ho usato continue / return per ridurre questo nidificazione ma ho anche sperimentato l'uso di questo stile con i cicli for, quindi:

foreach (var cart in carts)
{
    foreach (var item in cart.items)
    {
         foreach (var charge in item.charges)
         {
             ///...
         }
    }
}

diventa:

foreach (var cart in carts)
foreach (var item in cart.items)
foreach (var charge in item.charges)
{
    ///...
}

Mi sembra molto più facile da leggere.

C'è una buona ragione per non usare questo stile? È particolarmente raro / potrebbe causare problemi con altri sviluppatori dopo di me?

    
posta Joe 21.12.2017 - 12:41
fonte

4 risposte

8

Vedo un certo numero di problemi usando solo un set di parentesi:

Stai immediatamente rimuovendo la prima immagine mentale del loop a causa della mancanza di indentazione.

foreach (var cart in carts) {
    foreach (var item in cart.items) {
        foreach (var charge in item.charges) {
        }
    }
}

Questo mi dice "mi sto occupando di una iterazione 3D" e non ho nemmeno bisogno di leggere all'interno delle dichiarazioni di foreach per saperlo.

Anche l'indentazione mi dice immediatamente che sono nidificato. Se le dichiarazioni possono spesso sedersi l'una accanto all'altra:

if (condition1) {

}
if (condition2) {
   if (condition3) {

   }
}

Il annidamento è fondamentale nel dirmi come si relazionano tra loro. In modo simile, la nidificazione è fondamentale per dirmi anche come i due anelli interagiscono tra loro.

Che cosa succede se c'è qualcosa che vuoi solo fare in ogni ciclo ? Aggiungi le parentesi graffe per tutti e tre o solo i due che ne hanno bisogno? È una rottura di coerenza.

foreach (var cart in carts) {
    LogCartStatistics(cart);
    foreach (var item in cart.items) {
        LogItemPurchased(item);
        foreach (var charge in item.charges) {
            // Do the stuff
        }
    }
}

Se una riga viene aggiunta erroneamente nel mezzo in questo modo:

foreach (var cart in carts)
foreach (var item in cart.items)
LogDebug(item);
foreach (var charge in item.charges)
{}

Interromperà l'intero flusso di lavoro dei loop e non verrà compilato. Stai solo dando lavoro alla prossima persona che arriva, che probabilmente aggiungerà le parentesi e farà rientrare i loop, cosa che avrebbe potuto essere eseguita la prima volta.

Infine, e soprattutto, altri programmatori molto probabilmente non avranno familiarità con il tuo stile . Potrebbero anche considerarlo un errore (come direi). Per lo meno, hai violato il pensiero di qualcuno mentre cercano di identificare ciò che hai fatto, ciò che avresti dovuto fare e ciò che avevi intenzione di fare.

    
risposta data 21.12.2017 - 13:50
fonte
3

Per favore, non fare questo tipo di annidamento, usa linq.

foreach (var charge in carts.Select(c => c.items).Select(i => i.charges))
{
///...
}
    
risposta data 21.12.2017 - 12:56
fonte
0

Non vedo alcun motivo valido per usare questo modello, e non è comunemente usato, quindi anche se tu pensi che sia più leggibile, non sarà familiare ad altre persone che leggono il codice.

La rientranza nell'esempio intermedio chiarisce che stiamo davvero iterando in tre livelli, il che è importante per capire il codice. Vorrei sconsigliare di omettere le parentesi graffe per lo stesso motivo per cui consiglierei di omettere le parentesi graffe dopo if , while ecc.

A seconda di cosa stai effettivamente facendo all'interno del ciclo interno, un'espressione Linq potrebbe essere più idiomatica. foreach è appropriato se il blocco ha effetti collaterali, altrimenti un Select è più appropriato. Nel tuo esempio, i due anelli esterni non hanno effetti collaterali (poiché non hanno alcun codice accanto al ciclo interno), quindi Select è più appropriato per i due livelli esterni.

I've been refactorying some code recently that's suffering from messy nesting of if and for loops, I've been using continues/returns to reduce this nesting

Ti esorto a riconsiderare il tuo approccio. Sembra che tu non sia in realtà riducendo la complessità, ma piuttosto nascondendo la complessità usando i trucchi della sintassi.

    
risposta data 21.12.2017 - 13:02
fonte
-2

Il problema più ovvio:

se torno:

foreach (var cart in carts)
foreach (var item in cart.items)
foreach (var charge in item.charges)
{
   ///...
}

in:

foreach (var cart in carts)
var foo=true
foreach (var item in cart.items)
foreach (var charge in item.charges)
{
    ///...
}

now pippo è impostato su true molte volte, ma viene elaborato solo il carrello, se ne vengono elaborati.

    
risposta data 21.12.2017 - 12:59
fonte

Leggi altre domande sui tag