Does Scopeguard Use Really Lead to Better Code

Does ScopeGuard use really lead to better code?

It definitely improves your code. Your tentatively formulated claim, that it's obscure and that code would merit from a catch block is simply not true in C++ because RAII is an established idiom. Resource handling in C++ is done by resource acquisition and garbage collection is done by implicit destructor calls.

On the other hand, explicit catch blocks would bloat the code and introduce subtle errors because the code flow gets much more complex and resource handling has to be done explicitly.

RAII (including ScopeGuards) isn't an obscure technique in C++ but firmly established best-practice.

Where can I find a good Scope Guard implementation for my C++ projects?

ScopeGuard has been included in the Loki library (advertised in Modern C++ Design by Andrei Alexandrescu, I'm sure you've heard of this great book), and is mature enough to be used in production code, imo.

Just to be clear: We're talking about writing exception safe code using RAII.

Additional reading (on StackOverflow):
Does ScopeGuard use really lead to better code?

The simplest and neatest c++11 ScopeGuard

Boost.ScopeExit is a macro that needs to work with non-C++11 code, i.e. code that has no access to lambdas in the language. It uses some clever template hacks (like abusing the ambiguity that arises from using < for both templates and comparison operators!) and the preprocessor to emulate lambda features. That's why the code is longer.

The code shown is also buggy (which is probably the strongest reason to use an existing solution): it invokes undefined behaviour due to returning references to temporaries.

Since you're trying to use C++11 features, the code could be improved a lot by using move semantics, rvalue references and perfect-forwarding:

template< typename Lambda >
class ScopeGuard
{
    bool committed; // not mutable
    Lambda rollbackLambda; 
    public:

        // make sure this is not a copy ctor
        template <typename L,
                  DisableIf<std::is_same<RemoveReference<RemoveCv<L>>, ScopeGuard<Lambda>>> =_
        >
        /* see http://loungecpp.net/w/EnableIf_in_C%2B%2B11
         * and http://stackoverflow.com/q/10180552/46642 for info on DisableIf
         */
        explicit ScopeGuard(L&& _l)
        // explicit, unless you want implicit conversions from *everything*
        : committed(false)
        , rollbackLambda(std::forward<L>(_l)) // avoid copying unless necessary
        {}

        template< typename AdquireLambda, typename L >
        ScopeGuard( AdquireLambda&& _al , L&& _l) : committed(false) , rollbackLambda(std::forward<L>(_l))
        {
            std::forward<AdquireLambda>(_al)(); // just in case the functor has &&-qualified operator()
        }

        // move constructor
        ScopeGuard(ScopeGuard&& that)
        : committed(that.committed)
        , rollbackLambda(std::move(that.rollbackLambda)) {
            that.committed = true;
        }

        ~ScopeGuard()
        {
            if (!committed)
                rollbackLambda(); // what if this throws?
        }
        void commit() { committed = true; } // no need for const
};

template< typename aLambda , typename rLambda>
ScopeGuard< rLambda > // return by value is the preferred C++11 way.
makeScopeGuard( aLambda&& _a , rLambda&& _r) // again perfect forwarding
{
    return ScopeGuard< rLambda >( std::forward<aLambda>(_a) , std::forward<rLambda>(_r )); // *** no longer UB, because we're returning by value
}

template<typename rLambda>
ScopeGuard< rLambda > makeScopeGuard(rLambda&& _r)
{
    return ScopeGuard< rLambda >( std::forward<rLambda>(_r ));
}

C++11 scope exit guard, a good idea?

But is it a good idea?

Sure. A related topic is the RAII paradigm.

Or are there
potential problems I have missed?

You don't handle exceptions.

Is
there already a similar solution (with
C++0x features) in boost or similar?

Alexandrescu came up with ScopeGuard a long time back. Both Boost and std::tr1 has a thing called scoped_ptr and shared_ptr (with a custom deleter) that allows you to accomplish just this.

Is it safe lock/scope guard implementation?

Perhaps this program my help you visualize what happens:

#include <iostream>

// Replaces the int so that we know when things happen
struct Int {
   Int() {
       std::cout << "Int::Int()" << std::endl;
   }
   ~Int() {
       std::cout << "Int::~Int()" << std::endl;
   }
   Int(const Int &x) {
       std::cout << "Int::Int(const Int&)" << std::endl;
   }
};

struct LockGuard {
   LockGuard() {
     std::cout << "Locking" << std::endl;
   }
   ~LockGuard() {
     std::cout << "Unlocking" << std::endl;
   }    
};

struct A {    
  Int getValue() const {
     LockGuard lockGuard;
     return _value;
  }    
  Int _value;    
};

int main() {
   A a;
   std::cout << "about to call" << std::endl;
   Int x=a.getValue();
   std::cout << "done calling" << std::endl;
}

That results in

Int::Int()
about to call
Locking
Int::Int(const Int&)
Unlocking
done calling
Int::~Int()
Int::~Int()

One final note is that you might need to make _mutex mutable in your attribute declaration as locking and unlocking are typically non-const operation that would not be allowed in a const method otherwise.

Dynamically created scope guards

It seems you don't appreciate RAII for what it is. These scope guards are nice on occasion for local ("scope") things but you should try to avoid them in favour of what RAII is really supposed to do: encapsulating a resource in an object. The type FILE* is really just not good at that.

Here's an alternative:

void foo() {
    typedef std::tr1::shared_ptr<FILE> file_sptr;
    vector<file_sptr> bar;
    for (...) {
        file_sptr fsp ( std::fopen(...), std::fclose );
        bar.push_back(fsp);
    }
}

Or:

void foo() {
    typedef std::tr1::shared_ptr<std::fstream> stream_sptr;
    vector<stream_sptr> bar;
    for (...) {
        file_sptr fsp ( new std::fstream(...) );
        bar.push_back(fsp);
    }
}

Or in "C++0x" (upcoming C++ standard):

void foo() {
    vector<std::fstream> bar;
    for (...) {
        // streams will become "movable"
        bar.push_back( std::fstream(...) );
    }
}

Edit: Since I like movable types in C++0x so much and you showed interest in it: Here's how you could use unique_ptr in combination with FILE* without any ref-counting overhead:

struct file_closer {
    void operator()(FILE* f) const { if (f) std::fclose(f); }
};

typedef std::unique_ptr<FILE,file_closer> file_handle;

file_handle source() {
    file_handle fh ( std::fopen(...) );
    return fh;
}

int sink(file_handle fh) {
    return std::fgetc( fh.get() );
}

int main() {
    return sink( source() );
}

(untested)

Be sure to check out Dave's blog on efficient movable value types

How to avoid warning when using scope guard?

You can disable this warnings by -Wno-unused-variable, though this is a bit dangerous (you loose all realy unused variables).

One possible solution is to actually use the variable, but do nothing with it. For example, case it to void:

(void) g;

which can be made into a macro:

#define IGNORE_UNUSED(x) (void) x;

Alternatively, you can use the boost aproach: declare a templated function that does nothing and use it

template <typename T>
void ignore_unused (T const &) { }

...

folly::ScopeGuard g = folly::makeGuard([&] {close(sock);});
ignore_unused(g);

---

Related Topics