C++ – Using Lambdas to Improve Function Readability

clambdareadability

I'm looking to improve the readability of a lengthy C++ function. This function contains a number (> a dozen) variables that are used throughout. The main logic of the code is a long list of condition checks (sometimes nested, sometimes with loops) followed by various actions. Because of the lengthy number of lines detailing the actions to be performed mixed in with the logical/looping glue, it can be quite hard to follow what's going on. There's no simple way to siphon off these actions into separate functions without arbitrary, ugly function signatures noting whatever subset of variables happen to be needed. One drastic solution might be to replace the function with a singleton class, with the function's variables and sub-actions becoming private member variables and private member. However, I'm looking for a simpler alternative. My idea is to define a list of actions at the top of my function through lambda functions, and then below this perform the function's logic using these one-off lambdas. This would look something like (entirely schematically);

void function() {
    // variables
    int a, b, c, d, ...;

    // actions to be performed
    auto func1 = [&] () { ... code using a,b, etc. };
    auto func2 = [&] () { ... };
    auto func3 = [&] () { ... };
    ...

    // main logic
    if (<condition 1>) {
        if (<condition 2>)
            func1();
        else
            func2();
    } else {
        func2();
        func3();
    }
    ... // etc
}

These lambda functions would occasionally save code, in the the cases where they replace repeated code fragments, but would usually just improve readability — at least to my eyes. Is this a good practice in general? Do others find that this improves readability, and what is the cost of using these lambda functions?

Best Answer

Does it improve readability ?

Your way of using lambdas to break-down a larger function in smaller parts is similar to the nested functions in Pascal, ADA and other languages.

It indeed improves the readability of the main part of your function body: there are less statements to read to understand what it does. This is the main purpose of nested functions. Of course, I assume that nowadays, most programmers are familiar with the syntax of lambdas.

However, is it a good idea ?

Scott Meyers, in his book Effective Modern C++ warns against the use of default capture in lambdas. His main worry is about dangling references (e.g. if a lambda is defined in a block and is used out of the scope of this block when the variable doesn't exist anymore), which seems not to be an issue in your case.

But he also underlines another problem: the illusion of having a self-contained function. And here lies the major weakness of your approach:

you have the impression that your lambda is self contained, but in fact it's completely dependent of the rest of the code, and you don't see easily in your lambda where the captured values are coming from, which assumptions you can make on them, etc...
as the link with the main body is based on the captured variables, which can be read or written, it is in fact very difficult to guess all the side effects hidden in your lambda invocation, which could influence your main part.
so it's very difficult to identify assumptions and invariants in the code, both of the lambda, and of your mega function
in addition, you could accidentally change a variable that you forgot to declare locally in your lambda, and one happens to have the same name in the function.

First advice: at least, enumerate explicitly the variables captured by your lambda, in order to better control the potential side effects.

Second advice: once this works, you could think of strengthening your structure further, by evolving from capture to parameter passing. If there are too many of them, you'd have to refactor. One approach could be to make your function a callable class, promoting your throw away lambdas to member functions, and making the variables used throughout the computation member variables. But it's difficult to say if it's the best option from the elements you gave.

And why are you in such a situation ?

The next think to think about, is why you have such a big function in the first place. If you'd follow Uncle Bob's advice given in his book Clean Code (summary of the function topic on this blog page) you should have:

small functions,
that do one thing (single responsibility),
and that do only things at one level of abstraction

Related Solutions

Encapsulation in OOP – Using Lambdas for Better Encapsulation

I think the important point here is code locality: Code that should be read together should be close together.

My rule of thumb is: If both the called (lambda) function and the caller can be read, understood, maintained, tested (and possibly, but not necessarily, reused) separately, then extract the lambda function into a separate function or class. If the two are so closely tied together that you can't really make sense of one without knowing what the other does, then keep them as closely together as possible; lambdas are perfect for this, if the language you use doesn't allow nested functions.

Code Quality – Does Simplicity Always Improve Readability?

"Simple" is an overused word. "Readable" can profitably be defined as "simple to understand", in which case increasing (this measure of) simplicity by definition increases readability, but I don't think this is what you mean. I've written about this elsewhere, but generally something can be called "simpler" either by being more abstract (in which case fewer concepts can express more phenomena) or by being more concrete (in which case a concept does not require as much background knowledge to understand in the first place). I'm arguing that, depending on perspective, a more abstract concept can reasonably be called simpler than a more concrete concept, or vice versa. This, even though "abstract" and "concrete" are antonyms.

I'll use as an example some Haskell code I wrote a while ago. I asked a question on stackoverflow about using the List monad to calculate a counter in which each digit could have a different base. My eventual solution (not knowing much Haskell) looked like:

count :: [Integer] -> [[Integer]]
count [] = [[]]
count (x:xs) =
  -- get all possible sequences for the remaining digits
  let
    remDigits :: [[Integer]]
    remDigits = count xs
  in
  -- pull out a possible sequence for the remaining digits
  do nextDigits <- remDigits
     -- pull out all possible values for the current digit
     y <- [0..x]
     -- record that "current digit" : "remaining digits" is
     -- a valid output.
     return (y:nextDigits)

One of the answers reduced this to:

count = mapM (enumFromTo 0)

Which of these is "simpler" to understand (i.e. more readable) depends entirely on how comfortable the reader has become with (abstract) monadic operations (and, for that matter, point-free code). A reader who's very comfortable with these abstract concepts will prefer to read the (short) more abstract version, while one who is not comfortable with those operations will prefer to read the (long) more concrete version. There is no one answer about which version is more readable that will hold for everybody.