Are long functions acceptable if they have internal structure?

Question

When dealing with complicated algorithms in languages with support for nested functions (such as Python and D) I often write huge functions (because the algorithm is complicated) but mitigate this by using nested functions to structure the complicated code. Are huge (100+ line) functions still considered evil even if they're well-structured internally via the use of nested functions?

Edit: For those of you not familiar with Python or D, nested functions in these languages also allow access to the outer function scope. In D this access allows mutation of variables in the outer scope. In Python it only allows reading. In D you can explicitly disable access to the outer scope in a nested function by declaring it static.

Answer 1

Always remember the rule, a functions does one thing and does it well! If you can do so, avoid nested functions.

It hinders readability and testing.

Answer 2

Some have argued that short functions can be more error-prone than long functions.

Card and Glass (1990) point out that the design complexity really involves two aspects: the complexity within each component and the complexity of the relationships among components.

Personally, I've found that well-commented straight-line code is easier to follow (especially when you weren't the one who originally wrote it) than when it is broken up into multiple functions that are never used elsewhere. But it really depends on the situation.

I think the main take-away is that when you split up a block of code, you are trading one kind of complexity for another. There is probably a sweet spot somewhere in the middle.

Answer 3

Ideally, the entire function should be viewable without having to scroll. Sometimes, this isn't possible. But if you can break it up into pieces then it will make reading the code a lot easier.

I know that as soon as I push Page Up/Down or move to a different section of the code, I can only remember 7 +/- 2 things from the previous page. And unfortunately, some of those locations are going to be used when reading the new code.

I always like to think about my short-term memory like a computer's registers (CISC, not RISC). If you have the entire function on the same page, you can go to cache to get the required information from another section of the program. If the entire function cannot fit on a page, that would be the equivalent of always pushing any memory to disk after every operation.

Answer 4

Why use nested functions, rather than normal external functions?

Even if the external functions are only ever used in your one, formerly-big function, it still makes the whole mess easier to read:

DoThing(int x){
    x += ;1
    int y = FirstThing(x);
    x = SecondThing(x, y);
    while(spoo > fleem){
        x = ThirdThing(x+y);
    }
}

FirstThing(int x){...}
SecondThing(int x, int y){...}
ThirdThing(int z){...}

Answer 5

The answer is it depends, however you should probably turn that into a class.

Answer 6

I don't have the book in front of me right at this moment (to quote), but according to Code Complete the "sweetspot" for function length was around 25-50 lines of code according to his research.

There are times where is ok to have long functions:

• When the cyclomatic complexity of the function is low. Your fellow developers might get a little frustrated if they have to look at a function that contains a giant if statement and the else statement for that if is not on the screen at the same time.

The times where it's not ok to have long functions:

• You have a function with deeply nested conditionals. Do your fellow code readers a favor, improve the readability by breaking up the function. A function provides an indication to it's reader that "This is a block of code that does one thing". Also ask yourself if the length of the function indicates that it's doing too much and it needs to be factored out to another class.

The bottom line is that maintainability should be one of the highest priorities on your list. If another dev can't look at your code and get a "gist" of what the code is doing in less than 5 seconds, ur code doesn't provide enough "metadata" to tell what it's doing. Other devs should be able to tell what your class is doing just by looking at the object browser in your chosen IDE instead of reading 100+ lines of code.

Smaller functions have the following advantages:

• Portability: It's much easier to move functionality around (either within the class on refactoring to a different one)
• Debugging: When you look at the stacktrace it's much faster to pinpoint an error if you are looking at a function with 25 lines of code rather than 100.
• Readability - The name of the function tells what an entire block of code is doing. A dev on your team might not want to read that block if they aren't working with it. Plus, in most modern IDE's another dev can have a better understanding what your class is doing by reading the function names in an object browser.
• Navigation - Most IDE's will let you search on the name of functions. Also, most modern IDE's have the ability to view the source of a function in another window, this gives other devs to look at your long function on 2 screens (if the multi-monitors) instead of making them scroll.

The list goes on.....

Answer 7

No, multi-page functions are not desirable, and should not pass code review. I used to write long functions too, but after reading Martin Fowler's Refactoring, I stopped. Long functions are hard to write correctly, hard to understand and hard to test. I have never seen a function of even 50 lines that wouldn't be more easily understood and tested if it were split into a set of smaller functions. In a multi-page function there are almost certainly entire classes that should be factored out. It's hard to be more specific. Maybe you should post one of your long functions to Code Review and someone (maybe me) can show you how to improve it.

Answer 8

I don't like most nested functions. Lambdas fall in that category but usually don't flag me unless they have more than 30-40 characters.

The basic reason is that it becomes a highly locally dense function with internal semantic recursion, meaning that it's hard for me to wrap my brain around, and it's just easier to push some stuff out to a helper function that doesn't clutter the code space.

I consider that a function should Do Its Thing. Doing Other Things is what other functions do. So if you have a 200-line function Doing Its Thing, and it all flows, that's A-OK.

Answer 9

Is it acceptable? That's really a question only you can answer. Does the function achieve what it needs to? Is it maintainable? Is it 'acceptable' to the other members of your team? If so, then that's what really matters.

Edit: I didn't see the thing about the nested functions. Personally, I'd not use them. I'd use regular functions instead.

Answer 10

A long "straight-line" function can be a clear way to specify a lengthy sequence of steps that always occur in a particular sequence.

However, as others have mentioned, this form is prone to having local stretches of complexity in which the overall flow is less evident. Placing that local complexity into a nested function (ie: defined elsewhere within the long function, perhaps at top or bottom) can restore clarity to the mainline flow.

A second important consideration is controlling the scope of variables that are intended to be used only in a local stretch of a long function. Vigilance is required to avoid having a variable that was introduced in one section of code unwittingly referenced somewhere else (for example, after cycles of editing), as this kind of mistake will not show up as a compile or runtime error.

In some languages, this problem is easily averted: a local stretch of code can be wrapped in its own block, such as with "{...}", within which any newly-introduced variables are only visible to that block. Some languages, such as Python, lack this feature, in which case local functions can be useful to enforce smaller scope regions.

Answer 11

When I'm programming in python, i like to step back after I've written a function and ask myself if it adheres to the "Zen of Python" (type 'import this' in your python interpreter):

Beautiful is better than ugly.
Explicit is better than implicit.
Simple is better than complex.
Complex is better than complicated.
Flat is better than nested.
Sparse is better than dense.
Readability counts.
Special cases aren't special enough to break the rules.
Although practicality beats purity.
Errors should never pass silently.
Unless explicitly silenced.
In the face of ambiguity, refuse the temptation to guess.
There should be one-- and preferably only one --obvious way to do it.
Although that way may not be obvious at first unless you're Dutch.
Now is better than never.
Although never is often better than right now.
If the implementation is hard to explain, it's a bad idea.
If the implementation is easy to explain, it may be a good idea.
Namespaces are one honking great idea -- let's do more of those!

Answer 12

Put them in a separate module.

Assuming your solution is not more bloated than needing you haven't got too many options. You've already split up the functions in different sub functions so the question is where you should put it:

1. Put them in a module with only one "public" function.
2. Put them in a class with only one "public" (static) function.
3. Nest them in a function (as you've described).

Now both the second and third alternatives is in some sense nesting, but the second alternative would by some programmers not seem to bad. If you don't rule out the second alternative I don't see too much reason to rule out the third.