2

I'm looking for code shortening idea. I'm using boost::scoped_lock to lock a boost::mutex but I want to shorten the amount of code I'm writing.

Currently I have a mutex defined in my class and the member field called _sync. When I want to lock, I have to write:

scoped_lock<mutex> lock(_sync);

The tricky part is that this is a scoped lock, so I assume that if I write a static function to return the scoped_lock, then it will unlock as soon as it gets out of the function scope of the static function:

static scoped_lock<mutex> lock(mutex& sync)
{
    return scoped_lock<mutex>(sync);
}

This approach would make it really easy to type: 

public void Object::modify()
{
    lock(_sync); // <-- nice and short! ;)

    // do something to modify the object
    //..
    // the mutex is unlocked when we leave the scope of modify
}

Is my assumption correct? Will the scoped_lock unlock immediately when it's returned by my static function?

Kiril
  • 39,672
  • 31
  • 167
  • 226

3 Answers3

6

Don't ...

when you type scoped_lock<mutex> lock(_sync) everybody reading your code knows what is going on, and you will also if you look at your code two years from know. Laziness is usually a bad motivation to create implementation. Unless you want to enforce usage, have more than just hundreds of places where you need to write this expression, just don't do it

In the time it took you to write up the question, and the amount of time that you spent pondering how to do it, and the amount of time all of us took to answer your question you probably could have written all the scoped_lock<mutex> lock(_sync) that you needed. Especially if you are using your IDEs completions support.

Harald Scheirich
  • 9,676
  • 29
  • 53
  • Thanks for the upvote, not trying to be daft how would you improve it ? – Harald Scheirich Jul 21 '11 at 12:26
  • The reason why I want to do it this way is because I do a lot of my development in C# and 99% of the guys working here are working in C# too. In C# you call `lock(_sync){...}` and everybody knows what's going on, it's pretty simple. Having this shortcut makes looking at C++ code less "foreign" (and it's faster to type). – Kiril Jul 21 '11 at 14:53
4
#define LOCK(a) scoped_lock<mutex> scopedLockVar(a)

public void Object::modify()
{
    LOCK(_sync); // <-- nice and short! ;)

    // do something to modify the object
    //..
    // the mutex is unlocked when we leave the scope of modify
}

You should use a safe name for the define... The compiler just uses find and replace for defines...

Yochai Timmer
  • 48,127
  • 24
  • 147
  • 185
  • `__lockVar` is a [reserved identifier](http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier). – GManNickG Jul 20 '11 at 21:57
  • @GMan : Oops, didn't know that... changed it. Knew i should avoid 2 __ at the beginning of a variable name. – Yochai Timmer Jul 20 '11 at 21:59
  • 1
    @Yochai : Two consecutive underscores anywhere in the name, not just at the beginning, is illegal. – ildjarn Jul 20 '11 at 22:00
  • Just randomly naming a variable for the example... removed underscores altogether.. – Yochai Timmer Jul 20 '11 at 22:01
  • That all said, I think if you (@Lirik) find typing it out too much, if you're going to use a macro go all the way and just make a class-specific internal macro that's only visible during the implementation of the functions: `#define OBJECT_LOCK scoped_lock _scoped_lock_var_do_not_use_(_sync);` Then just throw `OBJECT_LOCK` at the type of all your functions. Simplest of all. It's not like you're going to be passing a different lock to the macro so no need having you type that too every time, nor should you try to make this some utility macro since it's so simple. – GManNickG Jul 20 '11 at 22:04
  • What if you want to lock two mutexes in the same scope? – Nemo Jul 20 '11 at 22:26
  • @Nemo: Then change the macro. – GManNickG Jul 20 '11 at 22:28
  • 1
    @nemo @gman `#define LOCK(a) scoped_lock scopedLockVar##a(a)` – Captain Obvlious Jul 20 '11 at 22:43
  • `#define LOCK(a,b) scoped_lock scopedLockVar(a); scoped_lock scopedLockVar(b)` – Yochai Timmer Jul 20 '11 at 23:13
  • @GMan the other reason I want to do this is to get a little bit closer to the C#/java style of locking, because we're a C# shop and most of the other developers have never touched C++. I hope that this way it will make it a little easier for them to follow the C++ code and it will make it easier to write code. – Kiril Jul 21 '11 at 22:05
3

I would have thought that your lock will be compiled away altogether.

The point of the scoped lock is that it exists locally, so you must declare it locally. I don't think there is a way of getting round this in c++ without macros (which just hides the code somewhere else).

I reckon use a typedef, then everyone knows what you are doing.

typedef scoped_lock<mutex> lock; //at the start of your class
lock l(_sync); //in your function.

I don't think this is so bad....

Tom
  • 5,219
  • 2
  • 29
  • 45
  • I had such a typedef once. But I kept misusing it like this: `lock(_sync)`, which locks and immediately unlocks. Then I created a macro just like Yochai Timmer. – Gabriel Sep 17 '12 at 17:13