Extra brace brackets in C++ code

Question

Sometimes you run into code that has extra brace brackets, that have nothing to do with scope, only are for readability and avoiding mistakes.

For example:

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

Other places I have seen it are:

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

This introduces a compiler error if the mutex isn't freed (assuming you remember both the } and the Release() call).

1. Is this a bad practice? Why?
2. If it isn't one, could it change how the code is compiled or make it slower?

Answer 1

The braces themselves are fine, all they do is limit scope and you won't slow anything down. It can be seen as cleaner. (Always prefer clean code over fast code, if it's cleaner, don't worry about the speed until you profile.)

---

But with respect to resources it's bad practice because you've put yourself in a position to leak a resource. If anything in the block throws or returns, bang you're dead.

Use Scope-bound Resource Management (SBRM, also known as RAII), which limits a resource to a scope, by using the destructor:

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&);
};

So you get:

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

Do this will all resources, it's cleaner and safer. If you are in a position to say "I need to release this resource", you've done it wrong; they should be handled automatically.

Answer 2

Braces affect variable scope. As far as I know that is all they do.

Yes, this can affect how the program is compiled. Destructors will be called at the end of the block instead of waiting until the end of the function.

Often this is what you want to do. For example, your GetMutexLock and ReleaseMutexLock would be much better C++ code written like this:

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
}

Using this more C++ style, the lock is released automatically at the end of the block. It will be released in all circumstances, including exceptions, with the exceptions of setjmp/longjmp or a program crash or abort.