Stile di programmazione: test di errore ricorrenti

Che ne dici di

 void bigFunction() {
      /* some computations */
      if(smallFunction1() == -1 || smallFunction2() == -1 || smallFunction3() == -1) {
           free(mem1);
           free(mem2);
           fclose(file);
           unlink(filename);
           return -1;
      }
      /* more computations and stuff in biggerFunction */
 }

O mi manca qualcosa qui?

Un'opzione consiste nel mettere il codice ripetuto nel proprio metodo:

void bigFunction() {
    /* some computations */
    if(smallFunction1() == -1) {
        CleanUpAfterError();
        return -1;
    }
    if(smallFunction2() == -1) {
        CleanUpAfterError();
        return -1;
    }
    if(smallFunction3() == -1) {
        CleanUpAfterError();
        return -1;
    }
    /* more computations and stuff in biggerFunction */
}

void CleanUpAfterError() {
    free(mem1);
    free(mem2);
    fclose(file);
    unlink(filename);
}

Questa è probabilmente l'unica circostanza in cui un goto sarebbe accettabile, ma puoi farlo senza.

void bigFunction() {
    /* some computations */
    bool has_error = smallFunction1() == -1;
    if (!has_error) {
        has_error |= smallFunction2() == -1;
    }
    if (!has_error) {
        has_error |= smallFunction3() == -1;
    }
    if(has_error) {
        free(mem1);
        free(mem2);
        fclose(file);
        unlink(filename);
        return -1;
    }
    /* more computations and stuff in biggerFunction */
}

In C questo è uno di quei casi in cui si può sostenere che l'uso giudizioso di goto è giustificabile, ad es.

void bigFunction()
{
    // NB: important to initialise these so that we can clean up properly in all cases
    void * mem1 = NULL;
    void * mem2 = NULL;
    FILE * file = NULL;

    /* some computations */

    if (smallFunction1() == -1)
        goto CleanUpAndExit;

    if (smallFunction2() == -1)
        goto CleanUpAndExit;

    if (smallFunction3() == -1)
        goto CleanUpAndExit;

    /* more computations and stuff in biggerFunction */

cleanUpAndExit:
    free(mem1); // NB: OK to call free on NULL pointer
    free(mem2);
    if (file != NULL) fclose(file); // NB: NOT OK to call fclose on NULL FILE *
    unlink(filename);
    return;
}

Naturalmente in altre lingue ci sono modi migliori per fare questo tipo di cose, ad es. eccezioni in C ++.

goto è generalmente disapprovato, e in 25 anni penso di aver dovuto usarlo una volta.

Tuttavia, questo tipo di cose assumono talvolta toni di dogma religioso e il pensiero razionale viene spinto da un lato.

Se un goto è il modo più semplice e più pulito per andare, allora usalo. Alla fine la chiarezza è buona. Torturare te stesso per evitare di usare un goto sta sprecando il tuo tempo prezioso e causando confusione al maintainer che segue.

In caso di dubbio, spiegare PERCHE 'l'hai fatto in questo modo con un commento.

Il mio standard di codifica dice che goto è proibito. Ma c'è anche una sezione in cui si dice che una delle regole può essere infranta per una buona ragione, a patto che la ragione venga spiegata nel codice sorgente con un commento. Ciò consente a tutto ciò che è pragmatico di essere fatto se è la cosa migliore per il compito. Gli standard di codifica dovrebbero essere relativi alle migliori pratiche, con una clausola "out" se ha senso.

La tua intuizione è giusta. Come suddividere la funzione dipende esattamente da cosa fanno i diversi bit (sentiti libero di postare altri dettagli). Ma in cima alla mia testa, le possibilità promettenti sarebbero:

• Se è possibile usare del codice C ++ in questo progetto, non usare malloc / free, invece di usare new e assegnarlo a un auto_ptr, e poi quando si "torna" e la variabile esce dall'ambito , la memoria verrà automaticamente cancellata. (Un approccio simile può essere utilizzato per eseguire automaticamente l'altro clean-up ogni volta che la variabile esce dall'ambito. La parola chiave con nome scomodo è "RAII: Acquisizione risorse è inizializzazione"

• Se è possibile utilizzare un codice C ++ in questo progetto, potrebbe essere opportuno o meno essere in grado di trasformare parte o tutta questa funzione in una classe, che accetta alcuni argomenti come argomenti del costruttore ed esegue il ripulitura sulla distruzione e il coraggio della funzione in una o più funzioni membro. (Potresti essere in grado di fare una cosa simile in C, inserendo le variabili in una struct, e avendo tutte le funzioni rilevanti prendere un puntatore a un'istanza di esso.)

• Suppongo che il codice clean-up si basi sulle variabili membro in bigFunction? (se no, sarebbe ragionevole scomporlo in una funzione separata). Ma potresti essere in grado di scomporre altre parti. Ad esempio, che dire di:

bool /* or int */ smallFunctionsOK( /* args */ ) {
    if (smallFunction1( /* args */ ) == -1) return false;
    if (smallFunction2( /* args */ ) == -1) return false;
    if (smallFunction3( /* args */ ) == -1) return false;
    return true;
}

void bigFunction() {
        someComputations( /* args */ );
     if(smallFunctionsOK()) {
          moreComputations( /* args */ );
     } else {
          free(mem1);
          free(mem2);
          fclose(file);
          unlink(filename);
          return -1;
     }
}

Probabilmente non puoi fare tutto questo perché alcune parti devono rimanere nella funzione principale, ma vedi se riesci a crearne alcune parti. Non abbiate paura di mettere i bit importanti in un'altra funzione e la noiosa pulizia in questo - è altrettanto probabile che leggerà facilmente come il contrario.

Esiste (almeno) un contesto in cui è possibile ripulire un po 'il codice ponendo l'onere di controllare lo stato sulle funzioni piccole anziché su quello grande. Questo dovrebbe essere usato solo in una situazione in cui le funzioni sono strettamente correlate e utilizzate in modo coerente in una successione simile.

Ogni piccola funzione prende un flag di stato come parte dei suoi parametri e lo controlla prima di procedere al suo calcolo.

void smallFunction1(..., bool* status)
{
    if(*status != True)
        return;

    // Computation...
}

La grande funzione viene quindi codificata come segue:

bool status = True;
smallFunction1(..., &status);
smallFunction2(..., &status);
smallFunction3(..., &status);
if(!status)
{
     // Clean up...
     return -1;
}

Certamente, c'è un'opportunità qui per qualche elaborazione dispendiosa, in cui vengono fatte diverse chiamate di funzione senza motivo. Pertanto, è consigliabile utilizzarlo nei casi in cui il tasso di errore previsto è basso.

Un esempio in cui questo tipo di cose potrebbe essere adatto sarebbe dove diversi campi di un pacchetto di dati sono codificati separatamente. Ad esempio, encodeHeader(...) , encodePayload(...) , encodeCRC(...) , ecc.

Diverse lingue lo gestiranno in modi diversi.

In C ++, la soluzione migliore è usare gli oggetti e ripulire i distruttori, usando RAII. Un vantaggio è che questo mantiene tutto il codice per qualcosa in un posto, piuttosto che avere tutto in una sezione di inizializzazione e di pulizia.

In alcune lingue, puoi inserire un try { ... } finally { ... } di costruzione per la pulizia.

C non ha nessuno di questi, quindi dovrai trovare il modo di fingere. Una tecnica è goto ( goto s non è così male se usato in modi limitati, le linee guida non ne usano mai una a meno che non si salti in avanti verso una sezione significativa di codice).