Is this a good pattern: replacing a long function with a series of lambdas?

Question

I recently run into the following situation.

class A{
public:
    void calculate(T inputs);
}

Firstly, A represents an object in the physical world, which is a strong argument for not splitting the class up. Now, calculate() turns out to be quite a long and complicated function. I perceive three possible structures for it:

• write it as a wall of text - advantages - all the information is in one place
• write private utility functions in the class and use them in calculate's body - disadvantages - the rest of the class doesn't know/care/understand about those methods

• write calculate the following way:

  void A::calculate(T inputs){    
      auto lambda1 = () [] {};    
      auto lambda2 = () [] {};    
      auto lambda3 = () [] {};
  
      lambda1(inputs.first_logical_chunk);
      lambda2(inputs.second_logical_chunk);
      lambda3(inputs.third_logical_chunk);
  }
  

Can this be considered a good or bad practice? Does this approach reveal any problems? All in all, should I consider this as a good approach when I am again faced with the same situation?

---

EDIT:

class A{
    ...
public:
    // Reconfiguration of the algorithm.
    void set_colour(double colour);
    void set_density(double density);
    void set_predelay(unsigned long microseconds);
    void set_reverb_time(double reverb_time, double room_size);
    void set_drywet(double left, double right);
    void set_room_size(double value);;

private:
    // Sub-model objects.
    ...
}

All those methods:

• get a value
• compute some other values, without using state
• call some of the "sub-model objects" to change their state.

It turns out that, excepting set_room_size(), those methods simply pass the requested value to sub-objects. set_room_size(), on the other hand, does a couple of screens of obscure formulas and then (2)does half a screen of calling sub-objects setters to apply the various obtained results. Therefore, I have separated the function into two lambdas and call them at the end of the function. Had I been able to split it into more logical chunks, I would have isolated more lambdas.

Regardless, the goal of the current question is to determine if that way of thinking should persist, or is it at best not adding value (readability, maintainability, debug-ability etc.).

Answer 1

No, this is not generally a good pattern

What you're doing is breaking up a function into smaller functions using lambda. However, there is a much better tool for breaking up functions: functions.

Lambdas work, as you have seen, but they mean much much much much much more than simply breaking up a function into local bits. Lambdas do:

• Closures. You can use variables in the outer scope inside the lambda. This is very powerful and very complicated.
• Reassignment. While your example make it hard to do assignment, one always has to pay attention to the idea that code may swap functions at any time.
• First-class functions. You can pass a lambda function to another function, doing something known as "functional programming."

The instant you bring lambdas into the mix, the next developer to look at the code must immediately mentally load all of those rules in preparation for seeing how your code works. They don't know that you're not going to use all that functionality. This is very expensive, compared to the alternatives.

It's like using a back-hoe to do your gardening. You know that you're only using it for digging little holes for this year's flowers, but the neighbors will get nervous.

Consider that all you are doing is grouping your source code visually. The compiler doesn't actually care that you curried things with lambdas. In fact, I'd expect the optimizer to immediately undo everything you just did when it compiles. You are purely catering to the next reader (Thanks for doing that, even if we disagree on methodology! Code is read far more often than it is written!). All you are doing is grouping functionality.

• Functions placed locally within the stream of source code would do just as well, without invoking lambda. Again, all that matters is that the reader can read it.
• Comments at the top of the function saying "we're dividing this function into three parts," followed by big long lines of // ------------------ between each part.
• You can also put each part of the calculation into its own scope. This has the bonus of immediately proving beyond all doubt that there is no variable sharing between parts.

EDIT: From seeing your edit with example code, I'm leaning towards the comment notation being the cleanest, with brackets to enforce the boundaries that the comments suggest. However, if any of the functionality was reusable in other functions, I'd recommend using functions instead

void A::set_room_size(double value)
{
    {
        // Part 1: {description of part 1}
        ...
    }
    // ------------------------
    {
        // Part 2: {description of part 2}
        ...
    }
    // ------------------------
    {
        // Part 3: {description of part 3}
        ...
    }
}

Answer 2

I think you made a bad assumption:

Firstly, A represents an object in the physical world, which is a strong argument for not splitting the class up.

I disagree with this. For example, if I had a class that represents a car, I would definitely want to split it up, because I surely want a smaller class to represent the tires.

You should split this function up into smaller private functions. If it really seems separated from the other part of the class, then that may be a sign that the class should be separated. Of course it is hard to tell without an explicit example.

I don't really see the advantage of using lambda functions in this case, because it does not really make the code cleaner. They were created to aid functional style programming, but this is not that.

What you wrote resembles a little bit to Javascript style nested function objects. Which is again a sign that they belong tightly together. Are you sure, that you shouldn't make a separate class for them?

To summarize, I do not think that this is a good pattern.

UPDATE

If you don't see any way to encapsulate this functionality in a meaningful class, then you can create file scoped helper functions, that are not members of your class. This is C++ after all, OO design is not a must.