Good or Bad C++ Idiom - Objects used purely for constructor/destructor?

Question

I have a few classes which do nothing except in their constructors/destructors. Here's an example

class BusyCursor 
{
  private:
    Cursor oldCursor_;

  public:

    BusyCursor()
    {
      oldCursor_ = CurrentCursor();
      SetCursor(BUSY_CURSOR);
    }
    ~BusyCursor()
    {
      SetCursor(oldCursor_);
    }
}

// example of use
    void DoSlowThing
    {
      BusyCursor busy;
      ... do something time-consuming  ...
    }

I'm a little concerned about future readability. Am I being too "tricksy" here, with a variable ("busy") which is never actually used in the code? Could some static analysis tool suggest they be removed, or is this idiom sufficiently common not to worry about?

Answer 1

This technique is very common and is known as the design pattern: Resource Acquisition Is Initialization (RAII).

I would not hesitate to use this design pattern at all.

It's much better that you are coding using this design pattern because you will avoid bugs by forgetting to reset the cursor, or whatever the resource in question is.

If you are concerned that other programmers might not understand it, then those programmers should be more educated. Always strive to code in the most error free way where you make it impossible for you and others to shoot yourself/themselves in the foot.

---

"Could some static analysis tool suggest they be removed?"

• No static analysis tool will see this as a problem.
• No compiler warning will be given
• No compiler optimization will cause any problems.

The reason is because the object is created and the constructor/destructor are called. So it is not an unreferenced variable.

Answer 2

It's a good idiom and commonly used.

It's better than any alternative, for example even if your something time-consuming code throws an exception, the ~BusyCursor destructor will still be called.

Answer 3

No sane static analysis tool would suggest removing the variable, becaues it is used. It has an effect because its constructor and destructor are called. You should be perfectly safe.

Answer 4

I usually refer to this as a "guard". In my opinion it demonstrates one of C++'s biggest strengths (deterministic resource-handling). It's one of the things I miss the most when working in garbage-collected languages.

Answer 5

Others have already mentioned that this is classic RAII. What I'll add is to say that this is one of the best things about C++. Very few other languages support it, or at least support it properly (even C#'s using construct isn't as good, as the burden is still on the client code - see my blog entry on this).

It's become so closely associated with C++ that you should feel confident that anyone reading it would be familiar with it - and if not they should be.

Answer 6

As others have said, this is good C++ style. To aid readability, I always prefix such RAII-only classes with Scoped (for example, ScopedBusyCursor) to make it clear from a glance what the class's purpose is.

Answer 7

Arguably not using this pattern is the bad idiom. When you are not using RAII your code ends up looking like this:

void func() {
    Cursor oldCursor = CurrentCursor();
    SetCursor(BUSY_CURSOR);
    try {
        do_slow_stuff();
        SetCursor(oldCursor);
    } catch (...) {
        SetCursor(oldCursor);
        throw;
    }
}

Do you really think having that littered throughout your code is better for maintenance?

Answer 8

This a well-known and good C++ idiom, like the others answered.

To make it clear that the classes are meant to be only used within a scope and not to be moved between different scopes, it might be good to make them noncopyable. This can be done manually by adding an unimplemented private copy-constructor and copy-assignment operator. The shorter and more readable way is to derive the class from boost::noncopyable:

#include <boost/noncopyable.hpp>
class BusyCursor : public boost::noncopyable // for scoped use only
{
    // ...
};

Answer 9

You can also go with something like ScopeGuard by Andrei Alexandrescu and Petru Marginean. Your sample would look something like this then:

void DoSlowThing
{     
    Cursor oldCursor = CurrentCursor();
    SetCursor(BUSY_CURSOR);
    ON_BLOCK_EXIT(SetCursor, oldCursor);
    ... do something time-consuming  ...
}

This makes it easier to do one-off RAII-type operations without having to create a new class for each one. However for your Cursor example, since it's a class you'll likely reuse many times, you're probably better off with the dedicated class.