current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 12 down vote favorite

Is using a static variable in a lambda function ok, or considered a bad practice? The code below works as intended (fills a vector with consecutive numbers).

#include <iostream>
#include <vector>
#include <algorithm>

using namespace std;

int main()
{
    vector<int> vec(100);

    generate(vec.begin(), vec.end(), [] () { static int i = 0; return i++; });
}
share|improve this question

migrated from stackoverflow.com Jul 27 '14 at 17:01

This question came from our site for professional and enthusiast programmers.
5  
@Avery, C++ is great in that if something appears to work, it still might not always work. That said, this example should just use std::iota. –  chris Jul 27 '14 at 16:24
    
@chris True, I was reading his question as a request to review his code - not as a question of future support (which I'm still not sure that he's asking about). –  Avery Jul 27 '14 at 16:29
    
@Avery - I asked about possible corner cases of using a global variable in lambda functions. –  w.b Jul 27 '14 at 16:47

6 Answers 6

up vote 12 down vote accepted

Yes it is perfectly valid.

lambdas in C++ were designed to be functionally equivalent to functors that were used a lot in C++03.

So you can consider:

auto x = [state1, state2](Param1 param1, Param2 param2){/* Do Stuff */};

To be functionally equivalent to: 

struct AnonClassX
{
    State1  state1;
    State2  state2;
    AnonClassX(State1 state1, State2 state2)
       : state1(state1)
       , state2(state2)
    {}
    returnValue operator()(Param1 param1, Param2 param2) const
    {
         /* Do Stuff */
    }
};
AnonClassX   x(state1,state2);

It is quite normal to use static variables in functions and methods. Lambda is just a shorthand for creating an anonymous class with state and an operator()() to make it act like a function. So it should be very normal to put static members inside it.

share|improve this answer
1  
Hmm, it feels to me that this is not really an answer to the question. Your examples lack static variables, and I don't think that static variables in functions are everyday occurences. –  phresnel Jul 28 '14 at 13:59
    
I believe there is an important difference between the two. In your functor, your variables would be local to any instantiated object. In the lambda, the static variable would be in the global space where static variables are stored. It may be valid C++, but I don't know if I would consider it a good coding practice. –  jliv902 Jul 28 '14 at 14:23
    
@jliv902: The words you are looking for are static storage duration and that answer is terrible. Whether using statics is good are bad is completely situational dependent. In this specific case I see no issues. The intent is clear. –  Loki Astari Jul 28 '14 at 14:33
1  
@phresnel: The example is not meant to show the usage of static. It is supposed to show the equivelence of lambda to functor. Then the last paragraph answers the question. Static function variables are a tool. They are used a lot. But like all tools they are subject to abuse. You have to look at their use in context. –  Loki Astari Jul 28 '14 at 14:37
    
@phresnel: Q1: Is the use in lambda OK: A1: Yes. –  Loki Astari Jul 28 '14 at 14:37
up vote 7 down vote

I would not do this since it is potentially confusing, e.g. what happens if you put that code in a function and use it multiple times to fill different vectors? I don't even know for sure if your code is actually correct.

Besides, std::iota already implements what you are doing, so in your specific case this construct is definitely not necessary.

share|improve this answer
1  
If it's in a function, I think the lambda is guaranteed to be a different type each time, each local to the function. I'm not 100% sure. This code is correct, though. One type is created for the lambda and its operator() has a static variable. –  chris Jul 27 '14 at 16:27
    
@chris Thanks, I didn't know that. But the fact that one has to think about it seems like a hint to avoid this construct if possible. –  Baum mit Augen Jul 27 '14 at 16:29
1  
@chris Each lambda has its own type, yes. But each lambda instance of given type (as in this case) share the same lambda type (obviously). –  c-smile Jul 27 '14 at 16:29
3  
@chris Clearly a new static variable cannot be created each time the lambda is generated because C++ has no facility for creating static variables at runtime. –  Raymond Chen Jul 27 '14 at 16:54
1  
@immibis Just a guess, but I don't think Raymond has an account on Code Review and this question was migrated from Stack Overflow. –  jliv902 Jul 28 '14 at 14:02
up vote 3 down vote

That's exactly what static variables are for - to keep some state between function calls.

share|improve this answer
up vote 4 down vote

Your code is fine. However, in this case I would use a closure to make the intended purpose more obvious to the reader:

#include <iostream>
#include <vector>
#include <algorithm>

using namespace std;

int main()
{
    vector<int> vec(100);

    int i = 0;
    generate(vec.begin(), vec.end(), [&i]() { return i++; });

    return 0;
}
share|improve this answer
    
Sorry but what these circumstances are exactly? –  c-smile Jul 27 '14 at 16:32
    
The usage of a static variable inside a lambda expression... –  Stefano Sanfilippo Jul 27 '14 at 16:33
    
How it is different from use of static variables inside normal functions? At least conceptually? –  c-smile Jul 27 '14 at 16:40
    
@c-smile no difference. Still, I tend to avoid local static variables whenever possible, since they make the functions stateful. Not really an issue here. Also, I find the closure version a bit more readable. –  Stefano Sanfilippo Jul 27 '14 at 17:07
2  
For one the lifetime of any function scope static is until end of main, so unless the compiler realizes the static can never be reached after the first use you'll still pay for it after returning. –  Ylisar Jul 27 '14 at 17:11
up vote 10 down vote

I wouldn't accept it in most of cases. Because if you do this:

vector<int> giveme() {
    vector<int> vec(100);

    generate(vec.begin(), vec.end(), [] () { static int i = 0; return i++; });
    return vec;
}

int main()
{
    auto a = giveme();
    auto b = giveme();
}

b would contain different values than a, and in my opinion, this is very far from obvious for the reader.

share|improve this answer
2  
That's more of a reflection on the function name giveme() than anything else. Self documenting names should help with intent. giveMeNext100ElementsOfIdSequence(). –  Loki Astari Jul 28 '14 at 14:44
up vote 2 down vote

This answer is late to the party, but I thought this needed to be said:

In most cases when you think static is a good idea for shared state, you should at least consider dependency injection instead.

In other words:

Is using a static variable in a lambda function ok, or considered a bad practice?

It is perfectly valid C++, but consider using an injected seed instead:

int main()
{
    vector<int> vec(100);

    int count = 0;
    // look ma', no static!
    generate(vec.begin(), vec.end(), [&count] () { return count++; });
}

In this (simplistic) example, the two are functionally equivalent. In practice though, static shares state between calls, even when you don't want it to (e.g. you may want to call this from multiple threads in the future, each with it's own count).

Both are acceptable and correct C++ (YMMV), but the [&count] alternative leads to less (hidden) side effects, more testable code, more re-usable code and a better habbit to cultivate in written code.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.