2014-05-16 10 views
6

ho incontrato un codice lungo le linee di:Apparentemente inutile #define della funzione

BOOL CBlahClass::SomeFunction(DWORD *pdw) 
{ 
    RETURN_FALSE_IF_FILE_DOESNT_EXIST 

    //the rest of the code makes sense... 
    //... 
} 

Tutto ciò che vedo rende abbastanza bene senso se non ho una piccola domanda circa la linea RETURN_FALSE_IF_FILE_DOESNT_EXIST

I cercato per questa stringa e trovo un # define:

#define RETURN_FALSE_IF_FILE_DOESNT_EXIST \ 
     if (FALSE==DoesFileExist()) return FALSE; 

la mia domanda è ... che diavolo? C'è qualche buona ragione per fare un #define come questo? Perché non basta scrivere:

BOOL CBlahClass::SomeFunction(DWORD *pdw) 
{ 
    if (FALSE == DoesFileExist()) return FALSE 

    //the rest of the code makes sense... 
    //... 
} 

L'unico motivo che mi viene in mente per fare questo è che è un po po più facile e po meno fastidioso per scrivere "RETURN_FALSE_IF_FILE_DOESNT_EXIST" poi a scrivere " if (FALSE == DoesFileExist()) restituisce FALSE ".

Chiunque vede altri motivi per farlo? C'è un nome per questo genere di cose?

+4

Un 'define' può essere facilmente modificato per fare qualcosa di diverso, come ad esempio l'uscita di un messaggio di errore,' exit' immediatamente, o anche non fare nulla. – usr2564301

+1

se questo è un vecchio codice (non hai specificato C o C++) #define sarebbe stato di fatto il modo delle funzioni inline –

+3

@imsoconfused Le funzioni inline non possono ottenere l'effetto di un'istruzione 'return', però. – aschepler

risposta

7

Bene, l'idea alla base dell'utilizzo della macro è semplificare la manutenzione in situazioni in cui questo controllo specifico potrebbe cambiare. Quando hai una macro, tutto ciò che devi modificare è la definizione della macro, invece di eseguire la scansione dell'intero programma e di modificare ogni controllo singolarmente. Inoltre, ti dà la possibilità di eliminare completamente questo controllo modificando la definizione della macro.

In generale, quando si ha una parte di codice ripetitiva piuttosto ben arrotondata, in genere si separa quel codice in una funzione. Tuttavia, quando quel pezzo di codice ripetitivo deve eseguire un'operazione non ortodossa, come return che chiude la funzione di inclusione, i macro sono fondamentalmente l'unica opzione. (One'd probabilmente d'accordo che i trucchi del genere dovrebbero essere riservati al debug/manutenzione/codice a livello di sistema, e deve essere evitato nel codice a livello di applicazione.)

+1

Semplifica anche 'grep'. –

+1

Immagino. Ma se il controllo sta per avere qualche cambiamento significativo avverrà nella funzione DoesFileExist, quindi sembra che non sia un granché un punto per #define. Immagino che questo possa davvero essere solo una questione di gusti ... – hft

+0

@hft: Sì, ma immagina di voler essere in grado di eliminarlo completamente. Ovviamente, può essere fatto facendo sì che 'DoesFileExist()' restituisca sempre 'TRUE'. Inoltre, in questo caso un compilatore intelligente potrebbe essere in grado di capire che l'intero 'if' può essere eliminato. Ma alcune persone si sentono intuitivamente meglio solo manualmente, assicurandosi che il controllo sia effettivamente eliminato invece di affidarsi a capacità di compilazione "avanzate". – AnT

4

quella macro è probabilmente un guard clause che asserisce una precondizione. Il presupposto è che un determinato file deve esistere perché non ha senso chiamare la funzione se non lo è. Probabilmente l'autore voleva che le clausole di guardia fossero notevolmente diverse dai controlli "regolari" e ne facessero una macro ovvia. Che sia una macro qui è semplicemente una preferenza di stile.

Si scrive una clausola di guardia quando si richiede che una condizione sia vera per continuare. In una funzione chiamata parseFile(), è possibile che il chiamante verifichi che il file esista. Ma in una funzione chiamata openFile() potrebbe essere perfettamente ragionevole che il file non esista ancora (e quindi non avresti una guardia).

Potrebbe essere più abituato a vedere assert(...). Ma cosa succede se non si desidera interrompere il programma quando la condizione è falsa e si desidera verificare la condizione anche in presenza di NDEBUG? Potresti implementare una macro come assert_but_return_false_on_fail(...), giusto? E questo è ciò che questa macro è. Un'alternativa assert().

glib è una libreria molto popolare che defines its precondition assertions come questo:

#define g_return_if_fail() 
#define g_return_val_if_fail() 
#define g_return_if_reached 
#define g_return_val_if_reached() 
#define g_warn_if_fail() 
#define g_warn_if_reached 

Lo stesso codice come la tua in glib sarebbe g_return_val_if_fail(DoesFileExist(), FALSE);.

Il codice incollato potrebbe essere migliore. Alcuni problemi sono:

  • È molto specifico. Perché non renderlo generico su qualsiasi condizione e non solo se esiste un file?

  • Il valore restituito è hardcoded nel nome.

  • Non esegue ulteriori operazioni di diagnostica. Le funzioni di glib registrano un errore piuttosto dettagliato in caso di errore, inclusa una traccia dello stack in modo da poter vedere la serie di chiamate che ha portato all'errore.

Questi problemi sono ciò che rende la macro sembrare sciocca e inutile. Sembrerebbe meno inutile altrimenti.

Oppure è possibile che l'autore fosse ossessionato dalle macro e ne sto leggendo troppo.

0

Non c'è motivo per questo. Un modo comune per cui i programmatori scrivono tali macro è perché hanno un codice con istruzioni a ritorno multiplo (che dovrebbero essere evitate se possibile), ma a volte non è semplicemente ragionevole evitarlo). Esempio:

// Bad code 

err_t func (void) 
{ 
    err_t err; 
    ... 

    if(err == error1) 
    { 
    cleanup(); 
    return err; 
    } 

    ... 

    if(err == error2) 
    { 
    cleanup(); 
    return err; 
    } 

    cleanup(); 
    return err; 
} 

Fare in modo che la pulizia di tutta la funzione sia una cattiva pratica, a causa della ripetizione del codice.

L'antico modo di risolvere il problema in modo più pulito sarebbe "su errore goto", in cui si inserisce un'etichetta alla fine della funzione. E fai pulizia e torna solo lì. In realtà sarebbe una soluzione un po 'accettabile, ma goto è un tabù, quindi le persone se ne allontanano. Invece sono venuti fuori con qualcosa di simile:

// Slightly less bad code 

#define RETURN_FROM_FUNCTION(err) { cleanup(); return (err); } 

err_t func (void) 
{ 
    err_t err; 
    ... 

    if(err == error1) 
    { 
    RETURN_FROM_FUNCTION(err); 
    } 

    ... 

    if(err == error2) 
    { 
    RETURN_FROM_FUNCTION(err); 
    } 

    RETURN_FROM_FUNCTION(err); 
} 

Questo rimuove il codice ripetizione, quindi il programma è più sicuro e più facile da mantenere. Questa è probabilmente la risposta alla tua domanda.

Ma questa macro non è ancora una buona soluzione, perché rende il codice più difficile da leggere, perché i programmatori C sono bravi a leggere C, ma sono pessimi leggendo il "linguaggio della casa, il linguaggio macro segreto". E naturalmente ci sono le solite preoccupazioni con i macro: tipo sicurezza, difficile da debug/maintain ecc.

Tuttavia, tutti i metodi sopra menzionati (incluso on-error-goto) provengono da un'incapacità di pensare fuori dagli schemi . La soluzione ottimale è questa:

// proper code 

err_t func (void) 
{ 
    err_t err; 

    allocate(); // allocate whatever needs to be allocated 

    err = func_internal(); 

    cleanup(); // one single place for clean-up 
    return err; 
} 


static err_t func_internal (void) // local file scope, private function 
{ 
    err_t err; 
    ... 

    allocate_if_needed(); // or maybe allocation needs to be here, based on results 

    ... 

    if(if(err == error1) 
    { 
    return err; 
    } 

    ... 

    if(err == error2) 
    { 
    return err; 
    } 

    return err; 
}