Usa un altro dopo l'eccezione (o meno)

9

Considera questo bit di codice:

if (x == 1)
{
  throw "no good; aborting" ;
}

[... more code ...]

Ora considera questo codice:

if (x == 1)
{
  throw "no good; aborting" ;
}
else
{
  [... more code ...]
}

I due casi funzionano esattamente allo stesso modo. Il primo caso ha il vantaggio che non è necessario "racchiudere" il resto del codice in else . Il secondo ha il vantaggio di seguire la pratica di avere esplicitamente else per ogni if .

Qualcuno può fornire argomenti solidi a favore degli uni sugli altri?

    
posta rlandster 30.10.2012 - 18:19
fonte

2 risposte

16

Non devi aggiungere else dopo if rami che interrompono il flusso di controllo incondizionatamente , come quelli che contengono throw o return . Ciò migliorerebbe la leggibilità del tuo programma rimuovendo il livello non necessario di nidificazione introdotto dal ramo else .

Ciò che appare più o meno OK con un singolo throw diventa davvero brutto quando vengono lanciati più tiri di fila:

void myMethod(int arg1, int arg2, int arg3) {
    // This is demonstrably ugly - do not code like that!
    if (!isValid(arg1)) {
        throw new ArgumentException("arg1 is invalid");
    } else {
        if (!isValid(arg2)) {
            throw new ArgumentException("arg2 is invalid");
        } else {
            if (!isValid(arg3)) {
                throw new ArgumentException("arg3 is invalid");
            } else {
                // The useful code starts here
            }
        }
    }
}

Questo snippet fa la stessa cosa, ma sembra molto meglio:

void myMethod(int arg1, int arg2, int arg3) {
    if (!isValid(arg1)) {
        throw new ArgumentException("arg1 is invalid");
    }
    if (!isValid(arg2)) {
        throw new ArgumentException("arg2 is invalid");
    }
    if (!isValid(arg3)) {
        throw new ArgumentException("arg3 is invalid");
    }
    // The useful code starts here
}
    
risposta data 30.10.2012 - 18:28
fonte
5

Chiamerei la pratica "esplicita" a cui fai riferimento come anti-pattern, poiché oscura il fatto che non esiste un codice caso speciale come un altro per il tuo if.

La leggibilità / manutenibilità è generalmente migliorata quando non si hanno per lo più nient'altro che costrutti di flusso del codice necessari e li si minimizza. Ciò significa che i nomi ridondanti e if che aggiungono un ambito a un'intera funzione rendono più difficile seguirlo e mantenerlo.

Ad esempio, hai questa funzione:

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (_validColors.Contains(oblogonToConfigure.Color))
    {
        oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
    }
    else
    {
        oblogonToConfigure.Color = _validColors[0];
        oblogonToConfigure.ColorIndex = 0;
    }
}

Ora viene richiesto che durante la configurazione sia necessario specificare l'indice tipo / tipo di oblogon, ci sono più ambiti che qualcuno potrebbe inserire quel codice e finire con codice non valido i.e.

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (!_validOblogons.Contains(oblogonToConfigure.Type))
    {
        oblogonToConfigure.Type = _validOblogons[0];
        oblogonToConfigure.TypeIndex = 0;
        if (_validColors.Contains(oblogonToConfigure.Color))
        {
            oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
        }
        else
        {
            oblogonToConfigure.Color = _validColors[0];
            oblogonToConfigure.ColorIndex = 0;
        }
    }
    else
    {
        oblogonToConfigure.TypeIndex = _validOblogons.IndexOf(oblogonToConfigure.Type);
    }
}

Confrontalo con se il codice originale è stato scritto con costrutti di flusso di controllo minimi necessari e minimizzati a quello.

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (!_validColors.Contains(oblogonToConfigure.Color))
    {
        oblogonToConfigure.Color = _validColors[0];
    }

    oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
}

Ora sarebbe molto più difficile mettere accidentalmente qualcosa nel campo di applicazione sbagliato o finire con scopi di gonfiamento che causano la duplicazione nella crescita a lungo termine e nel mantenimento di questa funzione. Inoltre è ovvio quali sono i possibili flussi attraverso questa funzione, quindi la leggibilità è migliorata.

Lo so, l'esempio è un po 'forzato, ma ho visto molte volte

SomeFunction()
{
    if (isvalid)
    {
        /* ENTIRE FUNCTION */
    }
    /* Nothing should go here but something does on accident, and an invalid scenario is created. */
}

Quindi formalizzare quelle regole sui costrutti del flusso di controllo penso che possa aiutare le persone a sviluppare l'intuizione necessaria per sentire l'odore di qualcosa quando iniziano a scrivere un codice del genere. Quindi inizieranno a scrivere ..

SomeFunction()
{
    if (!isvalid)
    {
        /* Nothing should go here, and it's so small no one will likely accidentally put something here */
        return;
    }

    /* ENTIRE FUNCTION */
}
    
risposta data 30.10.2012 - 18:42
fonte

Leggi altre domande sui tag