I'm attempting to write a simple ScopeGuard based on Alexandrescu concepts but with c++11 idioms.
namespace RAII
{
template< typename Lambda >
class ScopeGuard
{
mutable bool committed;
Lambda rollbackLambda;
public:
ScopeGuard( const Lambda& _l) : committed(false) , rollbackLambda(_l) {}
template< typename AdquireLambda >
ScopeGuard( const AdquireLambda& _al , const Lambda& _l) : committed(false) , rollbackLambda(_l)
{
_al();
}
~ScopeGuard()
{
if (!committed)
rollbackLambda();
}
inline void commit() const { committed = true; }
};
template< typename aLambda , typename rLambda>
const ScopeGuard< rLambda >& makeScopeGuard( const aLambda& _a , const rLambda& _r)
{
return ScopeGuard< rLambda >( _a , _r );
}
template<typename rLambda>
const ScopeGuard< rLambda >& makeScopeGuard(const rLambda& _r)
{
return ScopeGuard< rLambda >(_r );
}
}
Here is the usage:
void SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptions()
{
std::vector<int> myVec;
std::vector<int> someOtherVec;
myVec.push_back(5);
//first constructor, adquire happens elsewhere
const auto& a = RAII::makeScopeGuard( [&]() { myVec.pop_back(); } );
//sintactically neater, since everything happens in a single line
const auto& b = RAII::makeScopeGuard( [&]() { someOtherVec.push_back(42); }
, [&]() { someOtherVec.pop_back(); } );
b.commit();
a.commit();
}
Since my version is way shorter than most examples out there (like Boost ScopeExit) i'm wondering what specialties i'm leaving out. Hopefully i'm in a 80/20 scenario here (where i got 80 percent of neatness with 20 percent of lines of code), but i couldn't help but wonder if i'm missing something important, or is there some shortcoming worth mentioning of this version of the ScopeGuard idiom
thanks!
Edit I noticed a very important issue with the makeScopeGuard that takes the adquire lambda in the constructor. If the adquire lambda throws, then the release lambda is never called, because the scope guard was never fully constructed. In many cases, this is the desired behavior, but i feel that sometimes a version that will invoke rollback if a throw happens is desired as well:
//WARNING: only safe if adquire lambda does not throw, otherwise release lambda is never invoked, because the scope guard never finished initialistion..
template< typename aLambda , typename rLambda>
ScopeGuard< rLambda > // return by value is the preferred C++11 way.
makeScopeGuardThatDoesNOTRollbackIfAdquireThrows( 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 aLambda , typename rLambda>
ScopeGuard< rLambda > // return by value is the preferred C++11 way.
makeScopeGuardThatDoesRollbackIfAdquireThrows( aLambda&& _a , rLambda&& _r) // again perfect forwarding
{
auto scope = ScopeGuard< rLambda >(std::forward<rLambda>(_r )); // *** no longer UB, because we're returning by value
_a();
return scope;
}
so for completeness, i want to put in here the complete code, including tests:
#include <vector>
namespace RAII
{
template< typename Lambda >
class ScopeGuard
{
bool committed;
Lambda rollbackLambda;
public:
ScopeGuard( const Lambda& _l) : committed(false) , rollbackLambda(_l) {}
ScopeGuard( const ScopeGuard& _sc) : committed(false) , rollbackLambda(_sc.rollbackLambda)
{
if (_sc.committed)
committed = true;
else
_sc.commit();
}
ScopeGuard( ScopeGuard&& _sc) : committed(false) , rollbackLambda(_sc.rollbackLambda)
{
if (_sc.committed)
committed = true;
else
_sc.commit();
}
//WARNING: only safe if adquire lambda does not throw, otherwise release lambda is never invoked, because the scope guard never finished initialistion..
template< typename AdquireLambda >
ScopeGuard( const AdquireLambda& _al , const Lambda& _l) : committed(false) , rollbackLambda(_l)
{
std::forward<AdquireLambda>(_al)();
}
//WARNING: only safe if adquire lambda does not throw, otherwise release lambda is never invoked, because the scope guard never finished initialistion..
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()
}
~ScopeGuard()
{
if (!committed)
rollbackLambda();
}
inline void commit() { committed = true; }
};
//WARNING: only safe if adquire lambda does not throw, otherwise release lambda is never invoked, because the scope guard never finished initialistion..
template< typename aLambda , typename rLambda>
ScopeGuard< rLambda > // return by value is the preferred C++11 way.
makeScopeGuardThatDoesNOTRollbackIfAdquireThrows( 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 aLambda , typename rLambda>
ScopeGuard< rLambda > // return by value is the preferred C++11 way.
makeScopeGuardThatDoesRollbackIfAdquireThrows( aLambda&& _a , rLambda&& _r) // again perfect forwarding
{
auto scope = ScopeGuard< rLambda >(std::forward<rLambda>(_r )); // *** no longer UB, because we're returning by value
_a();
return scope;
}
template<typename rLambda>
ScopeGuard< rLambda > makeScopeGuard(rLambda&& _r)
{
return ScopeGuard< rLambda >( std::forward<rLambda>(_r ));
}
namespace basic_usage
{
struct Test
{
std::vector<int> myVec;
std::vector<int> someOtherVec;
bool shouldThrow;
void run()
{
shouldThrow = true;
try
{
SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptionsUsingScopeGuardsThatDoesNOTRollbackIfAdquireThrows();
} catch (...)
{
AssertMsg( myVec.size() == 0 && someOtherVec.size() == 0 , "rollback did not work");
}
shouldThrow = false;
SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptionsUsingScopeGuardsThatDoesNOTRollbackIfAdquireThrows();
AssertMsg( myVec.size() == 1 && someOtherVec.size() == 1 , "unexpected end state");
shouldThrow = true;
myVec.clear(); someOtherVec.clear();
try
{
SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptionsUsingScopeGuardsThatDoesRollbackIfAdquireThrows();
} catch (...)
{
AssertMsg( myVec.size() == 0 && someOtherVec.size() == 0 , "rollback did not work");
}
}
void SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptionsUsingScopeGuardsThatDoesNOTRollbackIfAdquireThrows() //throw()
{
myVec.push_back(42);
auto a = RAII::makeScopeGuard( [&]() { HAssertMsg( myVec.size() > 0 , "attempt to call pop_back() in empty myVec"); myVec.pop_back(); } );
auto b = RAII::makeScopeGuardThatDoesNOTRollbackIfAdquireThrows( [&]() { someOtherVec.push_back(42); }
, [&]() { HAssertMsg( myVec.size() > 0 , "attempt to call pop_back() in empty someOtherVec"); someOtherVec.pop_back(); } );
if (shouldThrow) throw 1;
b.commit();
a.commit();
}
void SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptionsUsingScopeGuardsThatDoesRollbackIfAdquireThrows() //throw()
{
myVec.push_back(42);
auto a = RAII::makeScopeGuard( [&]() { HAssertMsg( myVec.size() > 0 , "attempt to call pop_back() in empty myVec"); myVec.pop_back(); } );
auto b = RAII::makeScopeGuardThatDoesRollbackIfAdquireThrows( [&]() { someOtherVec.push_back(42); if (shouldThrow) throw 1; }
, [&]() { HAssertMsg( myVec.size() > 0 , "attempt to call pop_back() in empty someOtherVec"); someOtherVec.pop_back(); } );
b.commit();
a.commit();
}
};
}
}
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 ));
}