2010-08-25 33 views
12

A volte ci si imbatte in codice con parentesi graffe aggiuntive, che non hanno nulla a che fare con l'ambito, ma solo per la leggibilità e per evitare errori.Parentesi di parentesi graffa in codice C++

Ad esempio:

GetMutexLock(handle) ; 
{ 
    // brace brackets "scope" the lock, 
    // must close block/remember 
    // to release the handle. 
    // similar to C#'s lock construct 
} 
ReleaseMutexLock(handle) ; 

Altri luoghi che ho visto che sono:

glBegin(GL_TRIANGLES) ; 
{ 
    glVertex3d(..) ; 
    glVertex3d(..) ; 
    glVertex3d(..) ; 
} // must remember to glEnd! 
glEnd() ; 

Questo introduce un errore del compilatore se il mutex non viene liberato (assumendo che ricorda sia il} e il Chiamata Release()).

  1. È una cattiva pratica? Perché?
  2. Se non lo è, potrebbe cambiare il modo in cui il codice viene compilato o renderlo più lento?
+2

In OpenGL questa pratica ha lo scopo di migliorare la leggibilità del codice. Le parentesi su glBegin() chiama come delimitatore visivo del gruppo di vertici che dovrebbe essere disegnato. – karlphillip

+0

È una pratica terribile! Perché chiunque userebbe i delimitatori di blocchi per far risaltare un blocco di codice è fuori di me. Posso solo immaginare tutte le micro-micro-ottimizzazioni che mancano ...tutto sacrificato per qualcosa al di là del punto come leggibilità. In che mondo sta arrivando ???? –

+1

@Noah, ti ho rinominato Sarcastic Cat. – bobobobo

risposta

30

Le parentesi stesse stanno bene, tutto ciò che fanno è limitare l'ambito e non rallenterai nulla. Può essere visto come più pulito. (Preferire sempre il codice pulito sopra codice veloce, se è più pulito, non preoccuparti per la velocità fino a quando il profilo.)


Ma per quanto riguarda le risorse è cattiva pratica perché ti sei messo in condizione di perdita di una risorsa. Se qualcosa nel blocco lancia o ritorna, sbatti sei morto.

Usa Scope-bound Resource Management (SBRM, noto anche come Raii), che limita una risorsa a un ambito, utilizzando il distruttore:

class mutex_lock 
{ 
public: 
    mutex_lock(HANDLE pHandle) : 
    mHandle(pHandle) 
    { 
     //acquire resource 
     GetMutexLock(mHandle); 
    } 

    ~mutex_lock() 
    { 
     // release resource, bound to scope 
     ReleaseMutexLock(mHandle); 
    } 

private: 
    // resource 
    HANDLE mHandle; 

    // noncopyable 
    mutex_lock(const mutex_lock&); 
    mutex_lock& operator=(const mutex_lock&); 
}; 

in modo da ottenere:

{ 
    mutex_lock m(handle); 
    // brace brackets "scope" the lock, 
    // AUTOMATICALLY 
} 

Questa operazione comporterà tutte le risorse, è più pulita e più sicura. Se sei in grado di dire "Ho bisogno di rilasciare questa risorsa", hai sbagliato; dovrebbero essere gestiti automaticamente.

+7

+1 per correttezza ... e questo nuovo acronimo SBRM che non conoscevo! Molto meglio di RAII per quanto riguarda l'espressione dell'intento. E non sembra nemmeno esserci una pagina di Wikipedia! –

3

Non è una cattiva pratica. Non rende nulla più lento; è solo un modo di strutturare il codice.

Fare in modo che il compilatore esegua il controllo degli errori & fare rispettare per voi è sempre una buona cosa!

+0

+1 per fare in modo che il compilatore lavori ... –

+2

sfortunatamente liberare la risorsa da soli è soggetto a errori ... –

1

Tutto ciò che migliora la leggibilità IMHO è una buona pratica. Se l'aggiunta di parentesi graffe aiuta con la leggibilità, quindi andare per esso!

L'aggiunta di ulteriori controventi non cambierà la modalità di compilazione del codice. Non rallenterà il funzionamento del programma.

+0

{I {penso {che {aggiungendo {parentesi {può {ridurre} leggibilità}}}}}} . Seriamente, le parentesi dicono "c'è una struttura di controllo qui". Quando non ce n'è, continuo a cercarlo comunque per un momento. Facendomi perdere la cognizione e una frazione di secondo in cerca di qualcosa che non c'è non è leggibilità. –

+0

@David - Non penso che l'OP abbia pensato di aggiungere un numero arbitrario di parentesi al codice come nel tuo esempio. Mi riferivo al modo in cui l'OP stava usando le parentesi graffe. Ovviamente l'aggiunta di parentesi graffe come nel tuo esempio non aiuta con la leggibilità. – Starkey

+0

L'utilizzo di parentesi graffe per qualsiasi scopo diverso dall'indicare una struttura di controllo non aiuta la leggibilità, per quanto mi riguarda. Questo non conta come una struttura di controllo. Usarli per limitare l'ambito di una risorsa allocata RAII è. –

17

I controventi influenzano l'ambito variabile. Per quanto ne so, questo è tutto ciò che fanno.

Sì, questo può influenzare la modalità di compilazione del programma. I distruttori saranno chiamati alla fine del blocco invece di aspettare fino alla fine della funzione.

Spesso questo è quello che vuoi fare. Ad esempio, il GetMutexLock e ReleaseMutexLock sarebbe molto meglio di codice C++ scritto così:

struct MutexLocker { 
    Handle handle; 
    MutexLocker(handle) : handle(handle) { GetMutexLock(handle); } 
    ~MutexLocker() { ReleaseMutexLock(handle); }  
}; 
... 
{ 
    MutexLocker lock(handle); 
    // brace brackets "scope" the lock, 
    // must close block/remember 
    // to release the handle. 
    // similar to C#'s lock construct 
} 

Usando questo più stile C++, il blocco viene rilasciato automaticamente alla fine del blocco. Verrà rilasciato in tutte le circostanze, comprese le eccezioni, ad eccezione di setjmp/longjmp o arresto anomalo del programma o interruzione.

+0

Questa è la strada giusta da percorrere. – Puppy

+12

Hai bisogno di nominare l'armadietto, o è solo un temporaneo che muore subito. E probabilmente dovresti cambiare i commenti dopo di esso per abbinare la nuova gestione delle risorse. – GManNickG

+0

La sintassi corretta è 'Handle di MutexLocker;', non 'MutexLocker (handle);'. – kennytm

2

Non farà alcuna differenza per il codice compilato, a parte chiamare tutti i distruttori alla fine di quel blocco anziché la fine del blocco circostante, a meno che il compilatore non sia completamente pazzo.

Personalmente, chiamerei cattiva pratica; il modo per evitare il tipo di errori che potresti fare qui è utilizzare la gestione delle risorse con scope (a volte chiamata RAII), non usare i promemoria tipografici suscettibili di errori. Vorrei scrivere il codice come qualcosa come

{ 
    mutex::scoped_lock lock(mutex); 
    // brace brackets *really* scope the lock 
} // scoped_lock destructor releases the lock 

{ 
    gl_group gl(GL_TRIANGLES); // calls glBegin() 
    gl.Vertex3d(..); 
    gl.Vertex3d(..); 
    gl.Vertex3d(..); 
} // gl_group destructor calls glEnd() 
1

Questo è molto più utile (IMHO) in C++ con distruttori di oggetti; i tuoi esempi sono in C.

Immaginate se hai fatto una classe MutexLock:

class MutexLock { 
private: 
    HANDLE handle; 
public: 
    MutexLock() : handle(0) { 
     GetMutexLock(handle); 
    } 

    ~MutexLock() { 
     ReleaseMutexLock(handle); 
    } 
} 

Poi si potrebbe bloccare ambito che a poco il codice che ne aveva bisogno, fornendo un nuovo perimetro con le parentesi:

1

Se stai inserendo il codice in parentesi graffe, dovresti probabilmente scomporlo nel suo metodo. Se si tratta di una singola unità discreta, perché non etichettarla e romperla funzionalmente? Ciò renderà esplicito ciò che fa il blocco, e le persone che in seguito leggeranno il codice non dovranno capire.

3

Il posizionamento specifico di { ... } nell'esempio originale serve esclusivamente come zucchero di formattazione, rendendo più evidente l'inizio di un gruppo di istruzioni correlate logicamente e il punto in cui finisce. Come mostrato nei tuoi esempi, non ha effetto sul codice compilato.

Non so cosa intendi con "questo introduce un errore del compilatore se il mutex non viene liberato". Questo semplicemente non è vero. Tale uso di { ... } non può e non introdurrà alcun errore del compilatore.

Se si tratta di una buona pratica è una questione di preferenze personali. Sembra OK In alternativa, è possibile utilizzare commenti e/o rientranze per indicare il raggruppamento logico delle istruzioni nel codice, senza alcun ulteriore { ... }.

Ci sono varie tecniche basate sugli obiettivi, alcune delle quali sono state illustrate dalle altre risposte qui, ma quello che hai nel tuo OP non sembra neanche lontanamente simile a quello. Ancora una volta, ciò che hai nel tuo OP (come mostrato) è puramente un'abitudine di formattazione della sorgente con il superfluo { ... } che non ha alcun effetto sul codice generato.

+0

Sembra che tu sia l'unico che ha commentato il problema "errore del compilatore" - ben osservato. La mia impressione è che il manifesto ha visto macro utilizzate in questo modo: begin_x // fare cose END_X dove il begin_x e sostituzioni END_X includono il proprio {e}, tale che un END_X mancante fa produrre un errore di compilazione. Roba brutta –

+0

@ Tony: Boost ne ha alcuni. Una volta ho avuto circa un centinaio di pagine di errori perché ne ho dimenticato uno. "Brutta roba" non inizia a descriverlo :) –

Problemi correlati