14

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.).

  • 2
    What is it you think using lambdas will get you that function calls won't? – Blrfl Jan 27 '15 at 12:13
  • 1
    Firstly, A represents an object in the physical world, which is a strong argument for not splitting the class up. Surely A represents data about an object that could exist in the physical world. You can have an instance of A without the real object and a real object without an instance of A, so treating them like they're one and the same is nonsensical. – Doval Jan 27 '15 at 12:42
  • @Blrfl, encapsulation - no one but calculate() will know about those sub-functions. – Vorac Jan 27 '15 at 13:11
  • If all of those calculations are relevant to just A, that's taking it a bit to the extreme. – Blrfl Jan 27 '15 at 13:27
  • 1
    "Firstly, A represents an object in the physical world, which is a strong argument for not splitting the class up." I was, unfortunately, told this when I started programming. It took me years to realize that it's a bunch of horse hockey. It's a terrible reason to group things. I can't articulate what are good reasons to group things (at least to my satisfaction), but that one is one you should discard right now. The end all, be all of "good code" is that it works right, is relatively easy to understand, and is relatively easy to change (i.e., changes don't have weird side effects). – jpmc26 Jan 27 '15 at 23:53
13

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}
        ...
    }
}
  • So it's not an objective number, but I would subjectively claim that the mere presence of lambda functions makes me pay about 10 times more attention to each line of code, because they have so much potential to get dangerously complicated, fast, and do so on an innocent looking line of code (like count++) – Cort Ammon Jan 27 '15 at 15:38
  • Great point. I see those as the few advantages of the approach with lambdas - (1)code locally adjacent and with local scope(this would be lost by file-level functions) (2)compiler ensures that no local variables are shared between code segments. So those advantages can be preserved by separating calculate() into {} blocks and declaring shared data at the calculate() scope. I thought that by seeing that the lambdas do not capture, a reader wouldn't be encumbered by the power of lambdas. – Vorac Jan 27 '15 at 16:00
  • "I though that by seeing the lambdas do not capture, a reader wouldn't be encumbered by the power of lambdas." That is actually a fair, but contentious statement, which strikes to the heart of linguistics. Words usually have connotations that extend beyond their denotations. Whether my connotation of lambda is unfair, or if you are rudely forcing people to follow the strict denotation is not an easy question to answer. In fact, it may be totally acceptable for you to use lambda that way at your company, and totally unacceptable at my company, and neither one actually has to be wrong! – Cort Ammon Jan 28 '15 at 4:02
  • My opinion on the connotation of lambda stems from the fact that I grew up on C++03, not C+11. I have spent years developing an appreciation for the specific places where C++ has been hurt by a lack of lambda, such as the for_each function. Accordingly, when I see a lambda that doesn't fit one of those easy-to-spot trouble cases, the first assumption I arrive at is that it will likely be used for functional programming, because it wasn't needed otherwise. For many developers, functional programming is a completely different mindset from procedural or OO programming. – Cort Ammon Jan 28 '15 at 4:09
  • Thanks for explaining. I am a novice programmer and am now glad that I asked - before the habit had built up. – Vorac Jan 28 '15 at 8:36
20

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.

  • Sounds reasonable, but it is difficult for me to imagine the implementation. File-scope class with static methods? Class, defined inside A (maybe even functor with all other methods private)? Class declared and defined inside calculate() (this looks very much like my lambda example). As a clarification, calculate() is one of a family of methods (calculate_1(), calculate_2() etc.) of which all are simple, just this one is 2 screens of formulas. – Vorac Jan 27 '15 at 14:54
  • @Vorac: Why is calculate() so much longer than all the other methods? – Kevin Jan 27 '15 at 14:56
  • @Vorac It is really hard to help without seeing your code. Can you also post it? – Gábor Angyal Jan 27 '15 at 15:00
  • @Kevin, the formulas for everything are given in the requirements. Code posted. – Vorac Jan 27 '15 at 15:25
  • 4
    I'd upvote this 100 times if I could. "Modeling Objects in the Real World" is the start of the Object Design death spiral. It's a giant red flag in any code. – Fred the Magic Wonder Dog Jan 27 '15 at 15:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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