HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Improve my little syntactic hack

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
improvesyntacticlittlehack

Problem

Current best practice for using a lock_guard looks like this:

// introduce scope to take the lock
{
    lock_guard lock(sync);    // sync is an accessible mutex object
    do_protected_stuff();     // may throw, but the mutex is released regardless
}
// no exception, mutex is released in normal flow


I don't like the scope being introduce without a statement. I think it detracts from readability, and I think it's ugly. It occurred to me that what I wanted was the equivalent of modern Python's with statement:

with Lock() as lock:          # corresponds to lock+mutex in C++
    do_protected_stuff()


I thought about it for a while and designed a syntax hack that's gotten me partway there:

```
#include
#include
#include
using std::cout;
using std::endl;

//=============================================
// This is the "novel" part
// See main() for usage
//=============================================
// a utility class
class with_lock_
{
public:
with_lock_(boost::mutex &mtx_) : lock(mtx_), new_lock(true) {}
operator bool() { bool ret = new_lock; new_lock = false; return ret; }
private:
boost::lock_guard lock;
bool new_lock;
};
// a macro to hide the gory details
#define with_lock_guard(SYNC) for (with_lock_ lk_(SYNC); lk_; )

//==============================================

// some global stuff for multi-thread action
// a mutex
boost::mutex sync;

// a value protected by the mutex
float zz = 0.0f;

// a thread function that will change the value and crow about it
void get_it_yourself()
{
boost::lock_guard lock(sync);
zz = 99999.99999f;
cout << endl << " --- MINE!!" << endl;
}

//==============================================

int main()
{
boost::thread runit;

//=============================================
// This is the intended use...
// a little syntactic hack, inspired by Python
//=============================================
with_lock_guard (sync)
{
runit = boost::thre

Solution

Comments on Design

I like the idea in general but usually when I introduce a lock in a scope, the scope is usually the whole method so it does not look that ugly:

void MyClass::myMethod()
{
    lock_guard lock(sync);    // sync is an accessible mutex object
    do_protected_stuff();     // may throw, but the mutex is released regardless
}


But lets assume that you have situations that make your style worth while in using:

Now my problem is that you have situations where things are not as they seem:

with_lock_guard (sync)
    runit = boost::thread(get_it_yourself);
    cout << "Starting... ";
    boost::this_thread::sleep(boost::posix_time::millisec(500));
    cout << "OK!  Z = " << zz << endl;


Only the first statement is inside the lock. Yet I get no compiler error (yes I know silly error but the idea is that you should design your class so that it is really hard to abuse even accidentally):

I would change the design slightly so that it used a lambda:

with_lock_guard(sync, []()
{
    runit = boost::thread(get_it_yourself);
    cout << "Starting... ";
    boost::this_thread::sleep(boost::posix_time::millisec(500));
    cout << "OK!  Z = " << zz << endl;
});


This way it will be harder to abuse accidentally.
Comments on code:

If you are going to have operator bool() conversion method then at least make it explicit so it can not accidentally be abused. But since you are calling this from your own macro I would have a method that you explicitly call. This makes the code easier to understand.

https://stackoverflow.com/questions/6242768/is-the-safe-bool-idiom-obsolete-in-c11

// Prefer to use explicit on bool to prevent accidental usage:
explict operator bool() { bool ret = new_lock; new_lock = false; return ret; }


But I would do this:

#define with_lock_guard(SYNC)   for (with_lock_ lk_(SYNC); lk_.MakeSureWeRunOnceOnly(); )

Code Snippets

void MyClass::myMethod()
{
    lock_guard lock(sync);    // sync is an accessible mutex object
    do_protected_stuff();     // may throw, but the mutex is released regardless
}
with_lock_guard (sync)
    runit = boost::thread(get_it_yourself);
    cout << "Starting... ";
    boost::this_thread::sleep(boost::posix_time::millisec(500));
    cout << "OK!  Z = " << zz << endl;
with_lock_guard(sync, []()
{
    runit = boost::thread(get_it_yourself);
    cout << "Starting... ";
    boost::this_thread::sleep(boost::posix_time::millisec(500));
    cout << "OK!  Z = " << zz << endl;
});
// Prefer to use explicit on bool to prevent accidental usage:
explict operator bool() { bool ret = new_lock; new_lock = false; return ret; }
#define with_lock_guard(SYNC)   for (with_lock_ lk_(SYNC); lk_.MakeSureWeRunOnceOnly(); )

Context

StackExchange Code Review Q#16328, answer score: 4

Revisions (0)

No revisions yet.