Sta usando la programmazione errata ELSE? [chiuso]

17

Spesso ho incontrato bug causati dall'utilizzo del costrutto ELSE . Un primo esempio è qualcosa sulla falsariga di:

If (passwordCheck() == false){
    displayMessage();
}else{
    letThemIn();
}

Per me questo urla un problema di sicurezza. So che passwordCheck è probabile che sia un booleano, ma non metterei la sicurezza delle mie applicazioni su di esso. Cosa succederebbe se fosse una stringa, int ecc?

Di solito cerco di evitare di usare ELSE , e invece di optare per due istruzioni IF completamente separate per verificare ciò che mi aspetto. Qualsiasi altra cosa viene ignorata O viene gestita in modo specifico.

Sicuramente questo è un modo migliore per prevenire l'insorgenza di bug / problemi di sicurezza nell'app.

Come fate a farlo?

    
posta billy.bob 20.12.2010 - 12:11
fonte

13 risposte

89

Il blocco else deve sempre consistere in ciò che desideri sia il comportamento predefinito.

Non è necessario evitarli, fai solo attenzione a usarli in modo appropriato.

Nel tuo esempio, lo stato predefinito dovrebbe essere quello di non consentire l'accesso. Un piccolo refactoring ti lascia con:

If (passwordCheck)
{
   letThemIn();
}
else
{
   displayMessage();
}

vale a dire. se la verifica della password funziona, lasciateli entrare, altrimenti è sempre valido mostrare un messaggio di errore.

Ovviamente puoi aggiungere ulteriori controlli alla tua logica utilizzando else if invece di istruzioni if completamente separate.

    
risposta data 20.12.2010 - 12:18
fonte
57

No, non c'è niente di sbagliato in ELSE . ELSE non è il nuovo GOTO . In effetti, l'utilizzo di due IF s anziché ELSE può portare a diversi problemi.

Esempio 1:

if (this(is(a(very())==complex(check())))) {
   doSomething();
}

if (!this(is(a(very())==complex(check())))) {
   doTheOtherThing();
}

Vedi la copia-incolla? Aspetta semplicemente il giorno in cui ne cambi uno e dimentichi l'altro.

Esempio due:

foreach(x in y) {
  if (first_time) {
    first_time = false;
    doSomething();
  }

  if (!first_time) {
    doTheOtherThing();
  }
}

Come puoi vedere, anche il secondo IF verrà eseguito per il primo elemento, perché la condizione è già cambiata. Nei programmi del mondo reale, tali errori sono più difficili da individuare.

    
risposta data 20.12.2010 - 12:29
fonte
18

C'è sempre un ELSE. Se scrivi

if(foo)
  bar();

in realtà scrivi

if(foo)
{
  bar();
}
else
{
   // do nothing
}

Qualunque cosa tu abbia inserito nel percorso ELSE è sotto la tua responsabilità.

    
risposta data 20.12.2010 - 12:26
fonte
13

Personalmente tendo ad evitare else il più possibile, ma non è per alcun problema di sicurezza .

Durante la lettura del codice, le dichiarazioni annidate rendono più difficile seguire la logica, perché è necessario ricordare quale serie di condizioni ci porterà. Per questo motivo, sono un grande fan dell'uscita anticipata:

if (checkPassword != OK) { displayMessage(); return; }

letThemIn();

Questo vale anche per for e while cicli in cui userò continue e break ogni volta che si evita un livello di indentazione.

Chris Lattner lo dice meglio di me negli standard di codifica LLVM .

    
risposta data 20.12.2010 - 12:54
fonte
5

Quindi sostituiscili,

If (passwordCheck == true)
{
     letThemIn();
}
else
{
     displayMessage();
}
    
risposta data 20.12.2010 - 12:18
fonte
3

Come Matthieu M., preferisco uscire in anticipo da blocchi altrimenti nidificati ... Illustra una programmazione ben difensiva (in caso di cattive condizioni, nessun punto per continuare). Molte persone non saranno d'accordo con noi, preferendo un unico punto di uscita; non è il punto del dibattito (credo).

Ora, certamente uso else quando ha senso, in particolare per alternative semplici e brevi. Come detto, duplicare il test è una perdita di tempo (programmatore e CPU), una fonte di confusione e, più tardi, di bug (quando uno viene modificato, non l'altro).
A volte aggiungo un commento sulla parte else , ricordando qual era la condizione (in particolare se la parte if è lunga, ad esempio nel codice precedente) o qual è l'alternativa.

Si noti che alcuni esponenti estremi della programmazione funzionale propongono di liberarsi completamente di if , a favore della corrispondenza dei pattern ... Un po 'troppo estremo per i miei gusti. : -)

    
risposta data 20.12.2010 - 14:00
fonte
2

Non c'è niente di sbagliato nell'uso di ELSE. Tuttavia può portare a un codice troppo complesso che è difficile da leggere e capire. Potrebbe indicare un cattivo design. Indica certamente ulteriori casi d'uso che dovranno essere testati.

Prova a rimuovere ELSE se puoi, ma non essere paranoico a riguardo. Steve McConnell chiama questo codice di linea retta in Code Complete. Cioè c'è un semplice percorso chiaro attraverso il tuo codice.

Approcci per provare il tuo particolare problema:

  • usa il polimorfismo. Ai confini del sistema convalidare le credenziali di sicurezza dell'utente. Se sono legittimi, restituire un oggetto di sessione, con accesso alle parti rilevanti del sistema o generare un'eccezione. Tuttavia questo potrebbe renderti il sistema più complesso. Quindi decidi cosa è più facile da capire e mantenere.

In generale, quanto segue potrebbe aiutare a ridurre ELSE nel tuo codice:

  • i buoni requisiti possono ridurre la necessità di tali decisioni nel codice. Potrebbe non essere necessario implementare il caso d'uso (altro).
  • design più chiaro. Massima coesione e minimizzazione dell'accoppiamento. Ciò garantisce che i componenti non duplicino le decisioni prese in altri componenti.
  • gestione delle eccezioni per gestire i casi di errore.
  • polimorfismo (vedi esempio sopra)
  • switch statement - questi sono ELSE glorificati ma sono migliori in determinate situazioni.
risposta data 20.12.2010 - 12:54
fonte
2

La tua ipotesi sul fatto che il codice sia una perdita di sicurezza potrebbe essere o meno a seconda della lingua che stai utilizzando. Nel codice C potrebbe essere un problema (in particolare perché in C un booleano è solo un int che è diverso da zero o zero) - ma nella maggior parte dei linguaggi strongmente tipizzati (ad esempio il controllo del tipo di runtime) se la variabile passwordCheck è stata dichiarata come un booleano, non c'è modo di assegnargli qualcos'altro. In realtà, tutto in un predicato if deve essere risolto in un valore booleano, indipendentemente dal fatto che si utilizzino gli operatori booleani o semplicemente si utilizzi il valore. Se sei riuscito ad avere un altro tipo di oggetto associato a passwordCheck , il runtime genererebbe qualche tipo di eccezione cast illegale.

Semplici costrutti / else sono più facili da leggere rispetto ai costrutti / if - e meno inclini a problemi involontari se qualcuno tenta di capovolgere il costrutto. Prendiamo lo stesso esempio per un secondo:

if(passwordCheck == false) {
    denyAccess();
}

if(passwordCheck) {
    letThemIn();
}

Il significato delle clausole mutuamente esclusive che si desidera eseguire sopra è perso. Questo è ciò che il costrutto if / else trasmette. Due rami di esecuzione che si escludono a vicenda, dove uno di loro sarà sempre attivo. Questa è una parte importante della sicurezza: assicurati che non ci sia modo di letThemIn dopo aver chiamato denyAccess .

Ai fini della chiarezza del codice, e allo scopo di garantire che le sezioni critiche siano più protette, dovrebbero essere all'interno della clausola primaria (la parte if ). Il comportamento predefinito non conforme dovrebbe essere nella clausola alternativa (la parte else ). Ad esempio:

if(passwordCheck) {
    letThemIn();
} else {
    denyAccess();
}

NOTA: lavorando con lingue diverse, ho sviluppato un habbit di codifica che aiuta a evitare la domanda "cosa succede se è una stringa?" Essenzialmente, è mettere la costante prima nell'espressione booleana. Ad esempio, invece di controllare passwordCheck == false sto verificando false == passwordCheck . Questo evita anche il problema di assegnazione accidentale possibile in C ++. Utilizzando questo approccio, il compilatore si lamenterà se digito = anziché == . In linguaggi come Java e C #, il compilatore considererebbe l'assegnazione nella clausola if come un errore, ma C ++ lo accetterà volentieri. Ecco perché tendo anche a fare il controllo nullo con null prima.

Se cambi di routine le lingue ponendo la costante per prima è molto utile. Tuttavia, nel mio team è opposto allo standard di codifica e il compilatore riesce comunque a cogliere questi problemi. Può essere difficile da interrompere.

    
risposta data 20.12.2010 - 14:20
fonte
1

Dire che usare else quando la programmazione è sbagliata è come dire che usare otherwise quando si parla è male.

Certo, possono essere usati entrambi in modi sbagliati, ma ciò non significa che debbano essere evitati solo perché hai commesso un errore che è successo includendoli. Non sarei sorpreso se molti bug dipendessero da un caso default mancante in un'istruzione switch .

    
risposta data 20.12.2010 - 13:38
fonte
1

Pensa a Else come bianco: elenca il flusso della tua applicazione. Verifica le condizioni che DOVREBBE consentire al flusso dell'applicazione di continuare e, se queste non vengono soddisfatte, viene eseguito il Else per risolvere il problema, interrompere l'esecuzione dell'applicazione o qualcosa di simile.

Else di per sé non è male, ma se lo usi male, puoi vedere effetti indesiderati.

Inoltre, riguardo alla tua affermazione su

"So che passwordCheck è probabile che sia un booleano, ma non metterei la sicurezza delle mie applicazioni su di esso."

Per i metodi che sviluppi, restituisci SEMPRE un tipo di dati. Sebbene PHP Core sia disseminato di codice che restituisce due o più tipi di dati, questa è una cattiva pratica in quanto fa congettura delle chiamate di funzione. Se devi restituire più di un tipo di dati, considera la possibilità di lanciare un'eccezione (trovo che questo sia spesso il motivo per cui vorrei restituire un altro tipo di dati - qualcosa è andato orribilmente, orribilmente sbagliato) o prendere in considerazione la ristrutturazione del codice in modo da poter restituisce solo un tipo di dati.

    
risposta data 20.12.2010 - 15:37
fonte
1

Prima di tutto. LOL! Non c'è ragione per evitare altro. La sua NON cattiva pratica in qualsiasi modo, forma o forma.

Se possibile, il codice dovrebbe essere

if(!IsLoggedIn) { ShowBadLoginOrNoAccessPage(); return }

Non ci sono due se c'è e non ha altro. Questo è quello che faccio in tutte le mie app tranne quella in cui lancio un'eccezione. L'eccezione è catturata nella mia funzione che controlla l'url per la pagina corretta da mostrare (o in alternativa posso mettere la cattura / controllo nella funzione di errore asp.net). Stampa una pagina generica che dice di non autorizzare o qualunque messaggio io usi nell'eccezione (controllo sempre il tipo di eccezione e imposta il codice di stato http).

-Edit- come mostrato nell'esempio di ammoQ due ifs è ridicolo. Davvero else è altrettanto buono o migliore di un if. Se qualcosa deve essere evitato (anche se io personalmente non lo faccio. Ma io uso di ritorno e di interrompo molto) come è stato detto che più percorsi di codice aumentano la probabilità di bug. Vedi Complessità ciclomatica

-Edit 2- Se sei preoccupato dell'uso di / else. Noterò anche che la mia preferenza è mettere il blocco di codice più corto in alto come

if(cond == false) {
    a()
    b()
    c()
    onetwothree()
}
else
{
    a()
    b()
    c()
    more()
    onetwothree()
    code()
    longer()
}

Piuttosto quindi

if(cond) 
{
    a()
    b()
    c()
    more()
    onetwothree()
    code()
    longer()
}
else
{
    a()
    b()
    c()
    onetwothree()
}
    
risposta data 20.12.2010 - 20:46
fonte
0

Mi piace impostare un valore predefinito prima dei condizionali quando posso. Mi sembra che sia un po 'più facile da leggere e un po' più esplicito, ma questa è solo una preferenza. Ho la tendenza a cercare di evitare condizioni negative nel mio codice. Non sono un grande fan del controllo di! Foo o false == foo e mi sento come se fosse un po 'l'equivalente condizionale di un negativo.

foo = bar;

if ('fubar' == baz) {
    foo = baz;
}

invece di ...

if ('fubar' == baz) {
    foo = baz;
} else {
    foo = bar;
}

Il blocco di codice precedente mi sembra un po 'più facile da leggere. Mi sembra più naturale avere una sorta di paranoia scettica sul mio codice. Impostare un valore predefinito indipendentemente da qualsiasi condizione mi fa sentire a mio agio: P

    
risposta data 06.01.2011 - 22:02
fonte
0

Direi che l'uso della logica di ramificazione di qualsiasi tipo dovrebbe essere evitato il più possibile. Anche se non c'è niente di sbagliato in ELSE o IF, ci sono molti modi per scrivere codice per minimizzare la necessità di usare qualsiasi logica di ramificazione. Non sto dicendo che la logica della ramificazione può essere completamente eliminata - sarà necessaria in alcuni punti - ma è possibile reimpostare il codice per eliminarne una buona parte. Nella maggior parte dei casi ciò migliorerà l'intelligibilità e l'accuratezza del tuo codice.

Come esempio, gli operatori ternari di solito sono anche dei buoni candidati:

If VIP Then 
  Discount = .25
Else
  Discount = 0
End If
Total = (1 - Discount) * Total

Uso di un approccio ternario:

Discount = VIP ? .25 : 0
Total = (1 - Discount) * Total

Gli operatori ternari spostano la diramazione verso destra in un buon modo.

    
risposta data 23.08.2012 - 18:30
fonte

Leggi altre domande sui tag