In another question, a popular answer suggested that large functions demonstrate poor form.
How large would you let a function get before you broke it up?
(This could be in lines of code or a more qualitative answer, your choice. This is probably a language-dependent question to some extent, so you might want to indicate that and how compactly you normally write code--IOCCC style or space-o-plenty?--in your answer.)
A good rule of thumb to follow is 1.5 screens of code. If you have to hit page up/page down more than once to see the whole thing, then it's too long. This means that if you have more vertical resolution, you have have longer functions.
Usually if it can't fit on my screen, it's a candidate for refactoring. But, screensize does vary, so I usually look for under 25-30 lines. It's pretty subjective, though. If it feels long, then it's too long.
There is no hard rule and it really comes down to judgement and experience.
A function should have a singular purpose that is easily defined.
When reading code for a single function, you should be able to remember (mostly) what it is doing from beginning to the end. If you get partway through a function and start thinking "what was this function supposed to be doing again?" then that's a sure sign that it's too long.
I wouldn't worry so much about lines of code as keeping your function to one single task or idea. It is true that a long function can still keep to a single task, but that is less of a smell and less important to split up then a function that is trying to do several different things.
One way to determine if a function is the correct size and scope is to write a unit test for it. If you cannot write a concise unit test for your function that is testing one thing, your function is probably doing too much.
I don't trust any lines of code measure for this answer.
I think you should make functions as small as you can make them, as long as they remain discrete sensible operations in your domain. If you break a function ab() up into a() and b() and it would NEVER make sense to call a() without immediately calling b() afterwards, you haven't gained much.
Perhaps it's easier to test or refactor, you might argue, but if it really never makes sense to call a() without b(), then the more valuable test is a() followed by b().
Make them as simple and short as they can be, but no simpler!
If you're forcing your functions to be no more than X lines just by breaking them up arbitrarily, then that's bad programming style. The function might fit on the screen, but if it looks like:
void HugeFunction()
{
HugeFunctionPart1();
HugeFunctionPart2();
HugeFunctionPart3();
HugeFunctionPart4();
HugeFunctionPart5();
}
it's no more readable than if you had written the whole thing out.
If you're losing track of where you are in a function, then it's probably too long. Look for lines of code that you can say "OK, it does this" and pull that out into a function.
15 lines of code is good, but it can get larger or shorter depending on what exactly you are doing. The most important thing is to keep all the sentences of a function at the same abstraction level. For example, this would be incorrect:
Foo result = doSomething();
String another = extractString(result);
int index = another.lastIndexOf('.');
if (index != -1) {
another = index.substring(0, index);
}
return another;
Note that the first two lines invoke methods that do something at a higher level than the four last lines of code. This is better:
Foo result = doSomething();
String another = extractString(result);
another = removeExtension(another);
return another;
EDIT: why this is the most important thing for me? If you try to keep everything in a method in the same abstraction level, you'll end up with small functions without thinking about it. If you start having more than, say, 30 lines of code, then you are probably doing to much stuff (and part of that stuff might be just little details). By abstracting those details in other methods, you reduce the function size. You just don't reduce a method's size because it's bad, you do it to improve readability and to make it easier to understand, and abstraction accomplishes both.
The maximum function size is set through the compiler of your language. What I means is that the function size is not a dogma. If you have a large switch that you can be read very easy then there is no need for break in multiple methods.
A method should be a logical unit. It should do some thing with few parameters. If your function has more parameters as lines then you should not break. That it can make sense for a new function if it has only 5 lines.
The code should be good readble.
A function should accomplish a single task, and do it well. If it does more than a single overall task, then it should be broken up. Let the name function (singluar) guide you.
KISS & DRY
Length may not be the prime issue - there may be situations where longer is better.
What I once read and I've agree with: If you are able to properly NAME your function, you're usually right. So "CalculateEndOfTheWorld()" is good, but "DoAllSortOfUserManagementStuff()" is bad, in that case you might want to refactor the function.
1 Function = 1 Task = 1 Name.
My problem with big functions is that I want to hold everything that's happening in the code, in my head all at once. That's really the key. It also means that we're talking about a moving target.
Dense perl should appear in shorter chunks than padded VB. And, worse, at only 38, I'm finding that while my abilities have matured in nice ways, by core ability to grok a bunch of code is diminishing and thus the size of code blocks I want to handle is shrinking.
Because the goal is usability, the one screen rule really does make sense even though you can point to seeming flaws like varying screen resolutions. If you can see it all at once without paging around the editor, you are very much more likely to handle it all as a block.
What if you're working on a team? I suppose the best thing for the team would be to determine the lowest common denominator and target that size. If you have someone with a short attention-span or an IDE set up displaying around 20 lines, that's probably a good target. Another team might be good with 50.
And yeah, whatever your target is, sometimes you'll go over. 40 lines instead of 25 when the function can't really be broken up reasonably is fine here and there. You and your partners will deal with it. But the 3K line single-function VB6 modules that exist in some of my employer's legacy suites are insane!
quick answer: a good rule of thumb is a method that is doing too many different things, and those things aren't that related, or the different tasks in the method are at different levels of detail.
Extract Method and Consolidate Duplicate Conditional Fragments are two useful refactorings.
... Sometimes you need to rerrange the code to spot similarities when refactoring.
I'm usually checking my Python code with pylint. I'm using the default configuration file which state that a function should have a maximum number of statements of 50.
I found this talk by Marcel Molina, Jr very interesting. It lays out 3 rules to judge code and in particular functions: proportional, integrity and clarity.
Break down the function until it's clear. When you're making coding standards, don't set a line limit (or even a character limit). Just aim for clarity and maintainablility.
One of the things that leads to long functions is repeating long chunks of code over and over with slight variations. If you find yourself typing the same thing, or copying and pasting a chunk of code, split that off into a function. Use generics or templates to abstract patterns where different types are being used in the same way. Some languages let you do even better and let you abstract syntax (mostly LISP variants). Even if you're language doesn't support these type of abstractions, code generators can be your friend. Any time that you write some piece of code more than once
Often long functions are the result of a long list of inherently sequential events. Usually these can be easily and naturally grouped into smaller, more focused functions. However, if you really do have a large number of dependant commands that need to run in a certain order, then do what you can to make that structure clear. You can refactor until each command can be represented in a single line of code, all formatted similarly to indicate the linearness of the code. If this is a sequence that changes often, you could build/use a workflow framework to move the long list of commands into a configuration file.
As short as humanely possible. Whenever in doubt, extract a method based on the logical grouping of the code. Most of my methods are at the most between 7-10 lines long.
As a rule of thumb, I'd say that any method that does not fit on your screen is in dire need of refactoring (you should be able to grasp what a method is doing without having to scroll. Remember that you spend much more time reading code than writing it).
~20 lines is a reasonable maximum, though.
Aside from method length, you should watch out for cyclomatic complexity i.e. the number of different paths that can be followed inside the method. Reducing cyclomatic complexity is as important as reducing method length (because a high CC reveals a method that is difficult to test, debug and understand).
Don't use method length as an absolute metric, though. Obfuscated one-liners don't do much for readability, even if they trim your line count...
Do developers go around worrying about this? I don't go around thinking; "Oh this method is 56 lines long, I have to shorten it down by atleast 6 lines".
I just strive to make my code as good and as readable as possible. That means short methods and methodnames that explaines themself (And offcource alot more)...
During my first years of study, we had to write methods/functions with no more than 25 lines (and 80 columns max for each line). Nowadays I'm free to code the way I want but I think being forced to code that way was a good thing ... By writing small methods/functions, you more easily divide your code into reusable components, it's easier to test, and it's easier to maintain.
Steve McConnell says in Code Complete:
A large percentage of routines in object-oriented programs will be accessor routines, which will be very short. From time to time, a complex algorithm will lead to a longer routine, and in those circumstances, the routine should be allowed to grow organically up to 100–200 lines.
Let issues such as the routine's cohesion, depth of nesting, number of variables, number of decision points, number of comments needed to explain the routine, and other complexity-related considerations dictate the length of the routine rather than imposing a length restriction per se.
That said, if you want to write routines longer than about 200 lines, be careful. None of the studies that reported decreased cost, decreased error rates, or both with larger routines distinguished among sizes larger than 200 lines, and you're bound to run into an upper limit of understandability as you pass 200 lines of code.
The Composed Method pattern:
Divide your program into methods that perform one identifiable task. Keep all of the operations in a method at the same level of abstraction. This will naturally result in programs with many small methods, each a few lines long.
IMHO any function or method should fit on your screen. You should divide any large method into smaller parts, eg.:
function()
{
// init stuff
while(...)
{
// loop stuff
}
// final stuff
}
should become
init()
{
// init stuff
}
loop(index)
{
// loop stuff
}
final()
{
// final stuff
}
function()
{
init()
while(...)
{
loop(index)
}
final()
}
BTW (I don't know why) usually my methods are shorter than functions. And I like it :)
I find that if a method satisfies all three criteria you mentioned (easier to read/test/maintain), then you found the “right” length of the method. “Right” does not necessarily mean short, long, 10 lines or 50 lines – it just means that it has the right length. This depends (as you already assumed) on the programming language or the domain.
I often end up writing C# methods with 10 - 30 lines. Sometimes I find longer methods suitable, when it’s easier to read/test/maintain.
Instead of making a line count the criteria for the right length of a method, I would ask, if I have achieved all three criteria (easier to read/test/maintain).
To quote Abraham Lincoln: Long enough to reach the ground.
In other words, rather than choosing an arbitrary limit, it seems more sensible to understand the reason behind this sort of rule, and write your code based on that.
The desire to keep methods short is a good one, for all the reasons you mention. In addition, from a strictly defensive point of view, by limiting the number of lines of code, you also reduce the chance of introducing bugs. At the very least, you've given them precious little space to hide.
On the other hand, being "clever" simply to reduce the line count is generally counter-productive. It's far better to break a long method into several, particularly if each fragment performs a complete, stand-alone function that can be re-used, than to play code golf for the sake of brevity.
/End (mis-)quote, from Einstein: Your code should be as simple as possible, but no simpler.
I prefer to try to keep everything in a function on the same level of abstraction and as short as possibly. Programming is all about managing complexity and short one purpose functions make it easier to do that.