Is it good style to use lambda functions to define very small helper functions?

Question

As a silly example, let's say I have a function int f(vector<int> v), and for some reason, I need to do a couple of operations on v several times in f. Instead of putting a helper function elsewhere (which could increase clutter and hurt readability), what are the advantages and disadvantages to doing something like this (efficiency, readability, maintainability, etc.):

int f(vector<int> v)
{
    auto make_unique  = [](vector<int> &v)
    {
        sort(begin(v), end(v));
        auto unique_end = unique(begin(v), end(v));
        v.erase(unique_end, end(v));
    };
    auto print_vector = [](vector<int> const &v)
    {
        copy(begin(v), end(v), ostream_iterator<int>(cout, " "));
        cout << endl;
    };

   make_unique (v);
   print_vector(v);
   // And then the function uses these helpers a few more times to justify making
   // functions...
}

Or is there some preferred alternative?

Answer 1

The advantage of such locally scoped functions is that they don’t pollute the surrounding code with “helper” definitions—all of the behaviour can be restricted to a single scope. And since they have access to the lexical scope of the surrounding function, they can be used to factor behaviour without passing many parameters.

You can also use them to create small DSLs for abstracting the mechanical details of a function, allowing you to change them later. You define constants for repeated values; why not do the same for code?

For a tiny example, a state machine:

vector<int> results;
int current;
enum { NORMAL, SPECIAL } state = NORMAL;

auto input   = [&]{ return stream >> current; }
auto output  = [&](int i) { results.push_back(i); };
auto normal  = [&]{ state = NORMAL; };
auto special = [&]{ state = SPECIAL; };

while (input()) {
    switch (state) {
    case NORMAL:
        if (is_special(current)) 
          special(); 
        else output(current);
        break;
    case SPECIAL:
        if (is_normal(current)) 
            normal();
        break;
    }
}

return results;

A disadvantage is that you may be unnecessarily hiding and specialising a generic function that could be useful to other definitions. A uniquify or print_vector function deserves to be floated out and reused.

Answer 2

Efficiency:
This is basically functions versus functors which is what a lambda is under the hood. functors can actually be faster, this is because they are easier to inline, not all compilers will not inline function pointers (though it is possible). The reason for this is that when you pass a function pointer the compiler only knows the type of the function whereas the functor has the whole function because the type is unique.

Readability:
This part is more or less opinion based. Function pointers IMO are pretty annoying, they have ugly types and are not as versatile as functors. For example functors can easily be passed around at runtime. On the other hand writing out a full function is more readable than a big lambda.

The last point I would like to make is that a lambda like a functor can have a state (unlike a function, unless you count static vars), with a lambda you can capture a variable. Because of the compiler inlining limitations it is probably better not to pass around function pointers.

So I would say for small functions passed to stdlib it's better to use lambdas, and for larger ones implement a functor, function pointers are really no longer idiomatic in c++11.

Unfortunately it seems to me the way you are using them is not good. The compiler would have no problem inlining those as a normal function. And a function called print_vector or make_unique is better suited to it's own real function as it could be used in many other places.

Maintainability:
In this case I would say lambda's have a low maintainability, as they cannot be reused outside the function and they look messy since they crowd the function they are in. So it would be better to define them outside.

Answer 3

C++ isn't meant to support laziness. Using lambdas instead of normal functions is no exception here.

This

int f(vector<int> v)
{
    sort(begin(v), end(v));
    v.erase(unique(begin(v), end(v)), end(v));
    copy(begin(v), end(v), ostream_iterator<int>(cout, " "));
    cout << endl;
    /* And then the function uses these helpers a few more times to justify making functions... */
}

is much more succinct, standard, and readable than this

int f(vector<int> v)
{
    auto make_unique  = [](vector<int> &v)
    {
        sort(begin(v), end(v));
        auto unique_end = unique(begin(v), end(v));
        v.erase(unique_end, end(v));
    };
    auto print_vector = [](vector<int> const &v)
    {
        copy(begin(v), end(v), ostream_iterator<int>(cout, " "));
        cout << endl;
    };

   make_unique (v);
   print_vector(v);
   /* And then the function uses these helpers a few more times to justify making functions... */
}

As a general rule of thumb, when your code itself isn't obscure (erase and sort and copy aren't obscure, they're standard C++ algorithms), adding more code only makes it worse.

Furthermore: there is no reason to restrict the type of v to vector<int> in so many places.
Use type inference wherever the type doesn't matter.

If you feel it should be a separate function, then make it a separate function entirely.
Don't use lambdas as a crutch for being lazy.