Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

In object-oriented programming, there is no exact rule on the maximum length of a method , but I still found these two qutes somewhat contradicting each other, so I would like to hear what you think.

In Clean Code: A Handbook of Agile Software Craftsmanship, Robert Martin says:

The first rule of functions is that they should be small. The second rule of functions is that they should be smaller than that. Functions should not be 100 lines long. Functions should hardly ever be 20 lines long.

and he gives an example from Java code he sees from Kent Beck:

Every function in his program was just two, or three, or four lines long. Each was transparently obvious. Each told a story. And each led you to the next in a compelling order. That’s how short your functions should be!

This sounds great, but on the other hand, in Code Complete, Steve McConnell says something very different:

The routine should be allowed to grow organically up to 100-200 lines, decades of evidence say that routines of such length no more error prone then shorter routines.

And he gives a reference to a study that says routines 65 lines or long are cheaper to develop.

So while there are diverging opinions about the matter, is there a functional best-practice towards determining the ideal length of a method for you?

share|improve this question

migrated from stackoverflow.com Feb 5 '12 at 10:48

This question came from our site for professional and enthusiast programmers.

9  
Functions should be easy to understand. The length should follow from that, depending on circumstance. –  Henk Holterman Feb 5 '12 at 10:31
34  
I think the real limit is at 53 lines. With an average line size of 32.4 characters. Seriously, there's no definitive answer. A 100 line method can be very clear and maintainable, and a 4 line method can be a nightmare to understand. Generally though, long methods tend to have too many responsibilities, and be harder to understand and maintain than smaller ones. I would think in terms of responsibilities, and try to have a single responsibility per method. –  JB Nizet Feb 5 '12 at 10:32
14  
There is a term in programming called “functional coherence”. The length of a function should be allowed to vary provided that its implementation still constitutes a single coherent unit of logic in your application. Arbitrarily splitting up functions to make them smaller is more likely to bloat your code and hurt maintainability. –  Douglas Feb 5 '12 at 10:34
6  
And, if you want to limit your functions’ complexity, you should measure their cyclomatic complexity, not their length. A switch statement with 100 case conditions is more maintainable than 10 levels of if statements nested within each other. –  Douglas Feb 5 '12 at 10:37
6  
Bob Martin's approach is from 2008, Steve Mc Connell`s from 1993. They have different philosophies about what "good code" is, and IMHO Bob Martin tries to aquire a much higher level of code quality. –  Doc Brown Feb 5 '12 at 15:45
show 8 more comments

11 Answers

up vote 46 down vote accepted

Functions should clearly be short, between 5-15 lines is my personal "rule of thumb" when coding in Java or C#. This is a good size for several reasons:

  • It fits easily on your screen without scrolling
  • It's about the conceptual size that you can hold in your head
  • It's meaningful enough to require a function in its own right (as a standalone, meaningful chunk of logic)
  • A function smaller than 5 lines is a hint that you are perhaps breaking the code up too much (which makes it harder to read / understand if you need to navigate between functions). Either that or your're forgetting your special cases / error handling!

But I don't think it is helpful to set an absolute rule, as there will always be valid exceptions / reasons to diverge from the rule:

  • A one-line accessor function that performs a type cast is clearly acceptable in some situations.
  • There are some very short but useful functions (e.g. swap as mentioned by user unknown) that clearly need less than 5 lines. Not a big deal, a few 3 line functions don't do any harm to your code base.
  • A 100-line function that is a single large switch statement might be acceptable if it is extremely clear what is being done. This code can be conceptually very simple even if it requires a lot of lines to describe the different cases. Sometimes it is suggested that this should be refactored into separate classes and implemented using inheritance / polymorphism but IMHO this is taking OOP too far - I'd rather have one big 40-way switch statement than 40 new classes to deal with.
  • A complex function might have a lot of state variables that would get very messy if passed between different functions as parameters. In this case you could reasonably make an argument that the code is simpler and easier to follow if you keep everything in a single large function (although as Mark rightly points out this could also be a candidate for turning into a class to encapsulate both the logic and state)

So basically, use common sense, stick to small function sizes in most instances but don't be dogmatic about it if you have a genuinely good reason to make an unusually big function.

share|improve this answer
5  
There's a technical/perf reason for short methods, too: JIT cache hits. Many smaller, reusable methods are more likely to have been called before. Oh and an extra diag benefit, StackTraces are more focused on the logic that went pop. –  Luke Puplett Aug 21 '13 at 10:32
3  
"use common sense" is the most important advice –  Simon Jun 18 at 10:10
add comment

I'll just throw in yet another quote.

Programs must be written for people to read, and only incidentally for machines to execute

-- Harold Abelson

It is very improbable that functions that grow to 100-200 follow this rule

share|improve this answer
    
Except when they contain a switch. –  Calmarius Jun 20 at 12:41
add comment

IMHO, you shouldn't have to use scrollbar to read your function. As soon as you need to move scrollbar, it take a few more time to understand how work the function.

Accordingly, it depends of usual programming environment of your team work (screen resolution, editor, font size, etc...). In 80's, it was 25 lines and 80 columns. Now, on my editor, I display nearly 50 lines. Number of columns I display did not change since I split my screen in two to display two files at times.

In brief, it depends of setup of your coworkers.

share|improve this answer
2  
Wasn't it rather 24 lines that time? I'm thinking of 3270 or 9750 terminals, where the 25th was the status line. And the terminal emulations followed this. –  ott-- Jul 5 '13 at 9:26
    
some systems/editors had 40 or 50 lines from the start. These days 150 lines is not uncommon and 200+ is doable, so that's not really a good metric. –  Ӎσᶎ Jun 5 at 5:46
    
I use my screen in portrait orientation, I can see 200 lines of code at once. –  Calmarius Jun 20 at 12:48
add comment

I keep functions as short as possible, but not at at all cost. Most of my functions have less than 15 lines. If I see, that I need to scroll to see the last line of the function, I break it.

share|improve this answer
2  
how does this answer the question asked? –  gnat Aug 21 '13 at 8:09
add comment

If I find long method - I could bet this method is not properly unit-tested or most time it hasn't unit test at all. If you start doing TDD you will never build up 100-lines methods with 25 different responsibilities and 5 nested loops. Tests oblige you constantly refactor your mess and write uncle's Bob clean code.

share|improve this answer
add comment

It depends, seriously, there really isn't a solid answer to this question because the language you are work with matters, the five to fifteenth lines mentioned in this answer might work for C# or Java, but in other languages it doesn't give you much to work with. Likewise, depending upon the domain you are working in, you might find yourself writing code setting values in a large data structure. With some data structures you might have tens of elements that you need to set, should you break things out in to separate functions just because your function is running long?

As others have noted, the best rule of thumb is that a function should be a single logical entity that handles a single task. If you try to enforce draconian rules that say that functions can't be longer than n lines and you make that value too small your code will grow harder to read as developers try and use fancy tricks to get around the rule. Likewise, if you set it too high it will be a non-issue and can lead to bad code though laziness. Your best bet is to just conduct code reviews to ensure that functions are handling a single task and leave it at that.

share|improve this answer
add comment

While I agree with other's comments when they said there's no hard rule about right LOC number, I bet if we look back at the projects we've looked at in the past and identify every function above, let's say 150 lines of code, I'm guessing we would come to a consensus that 9 out of 10 of those functions break SRP (and very likely OCP as well), have too many local variables, too much control flow and are generally hard to read and maintain.

So while LOC may not be a direct indicator of bad code, it is certainly a decent indirect indicator that certain function could be written better.

On my team I fell into the position of a lead and for whatever reason, people seem to be listening to me. What I generally settled on is to tell the team that while there's no absolute limit, any function more 50 lines of code should at a minimum raise a red flag during code review, so that we take a second look at it and re-evaluate it for complexity and SRP/OCP violations. After that second look, we might leave it alone or we might change it, but at least it makes people think about these things.

share|improve this answer
2  
This seems sensible - LOC doesn't mean anything with respect to complexity or code quality, but it can be a good marker for the possibility that things should be refactored. –  cori May 31 '12 at 19:18
add comment

There is no absolute rules about method's length, but the following rules have been useful:

  1. Function's primary purpose is to find the return value. There is no other reason for it's existence. Once that reason is fullfilled, no other code should be inserted to it. This necessarily keeps functions small. Calling other functions should only be done if it makes finding the return value easier.
  2. On the other hand, interfaces should be small. This means you either have large number of classes, or you have large functions -- one of the two is going to happen once you start to have enough code to do anything significant. Big programs can have both.
share|improve this answer
    
What about side effects - writing to a file, resetting status, etc. ? –  Vorac Aug 21 '13 at 10:46
add comment

I stepped into a project which hasn't had any care about coding guidelines. When I look into the code I sometimes find classes with more than 6000 lines of code and less than 10 methods. This is a horror scenario when you have to fix bugs.

A general rule of how big a method should be at maximum is sometimes not so good. I like the rule by Robert C. Martin (Uncle Bob): "Methods should be small, smaller than small". I try to use this rule all the time. I am trying to keep my methods simple and small by clarifying that my method does only one thing and nothing more.

share|improve this answer
1  
I don't see why a 6000 line function is harder to debug than a shorter one... Unless it has other problems too (eg. repetition) –  Calmarius Mar 14 '13 at 15:13
6  
It has nothing to do with debugging.. its about complexity and maintainability. How you would extend or change a 6000 lines method done by another one? –  Smokefoot Mar 15 '13 at 16:03
    
@Calmarius the difference is usually that 6000 line functions tend to contain local variables which were declared very far away (visually), making it difficult for the programmer to build up the mental context required to have high confidence about the code. Can you be sure about how a variable is initialised and built up at any given point? Are you sure nothing is going to mess with your variable after you've set it on line 3879? On the other hand, with 15 line methods, you can be sure. –  Daniel B Jun 20 at 10:31
    
@DanielB I wouldn't touch any variable before I know how it is used. So I would search for all occurrences of it in its scope to see which other functions and statements alter it. I'd also refactor the code to bring down all variables into the smallest scope possible. –  Calmarius Jun 20 at 12:32
    
@Smokefoot I would factor out repetitive patterns into single functions/macros etc. If the function isn't a repetitive mess, there is 90% chance the reason for its length is a huge switch, which mean you probably need to add just a new case. If you explode the 6000 lines into 60 100 line functions, you make the next fellow's job even harder because now he'll need to scroll and jump around many source files to put things together in his mind. –  Calmarius Jun 20 at 12:39
show 1 more comment

It is not about number of lines, it is about SRP. According to this principle, your method should do one and only one thing.

If your method does this AND this AND this OR that => it is probably doing to much. Try to look at this method and analyse: "here I get this data, sort it and get elements I need" and "here I process these elements" and "here I finally combine them in order to get the result". These "blocks" should be refactored to other methods.

If you simply follow SRP most of your method will be small and with clear intention.

It is not correct to say "this method is > 20 lines so it is wrong". It can be an indication that something may be wrong with this method, no more.

You may have a 400 lines switch in a method (often happens in telecom), and it is still single responsibility and it is perfectly OK.

share|improve this answer
2  
Large switch statements, output formatting, definitions of hashes/dictionaries that should be hardcoded rather than flexible in some database, this often happens, and is perfectly fine. As long as the logic is devided, then you're all good. A large method could prompt you to think 'should I split this'. The answer could very well be 'no, this is fine as it is' (or yes, this is an absolute mess) –  Martijn Feb 5 '12 at 15:16
    
What does "SRP" mean? –  thomthom Dec 10 '13 at 14:41
    
SRP stands for Single Responsibility Principle and states that every class (or method) should have only one responsibility. It is related to cohesion and coupling. If you follow the SRP, your classes (or methods) will have good cohesion, but coupling may be increased because you end up with more classes (or methods). –  Kristian Duske Jun 18 at 11:31
    
+1 for SRP. By writing cohesive functions, one can then more easily combine them in functional style to achieve more complex results. In the end it's better that a function be made up of three other functions that got glued together than to have a single function doing three discrete, even if somehow related, things. –  Mario T. Lanza Jun 20 at 12:05
add comment

I think one problem here is that the length of a function says nothing about its complexity. LOC (Lines of Code) are a bad instrument to measure anything.

A method should not be overly complex, but there are scenarios where a long method can be easily maintained. Note that the following example does not say it could not be split into methods, just that the methods would not change the maintainability.

for example a handler for incoming data can have a large switch statement and then simple code per case. I have such code - managing incoming data from a feed. 70 (!) numerically coded handlers. Now, one will say "use constants" - yes, except the API does not provide them and I like to stay close to "source" here. Methods? Sure - just sadly all of them deal with data from the same 2 huge structures. No benefit in splitting them off except maybe having more methods (readability). The code is intrinsically not complex - one switch, depending on a field. Then every case has a block that parses x elements of data and publishes them. No maintenance nightmare. There is one repeating" if condition that determiens whether a field has data (pField = pFields [x], if pField->IsSet() { blabla }) - same pretty much for every field...

Replace that with a much smaller routine containing nested loop and a lot of real switching statements and a huge method can be more easy to maintain than one smaller one.

So, sorry, LOC is not a good measurement to start with. If anything, then complexity / decision points should be used.

share|improve this answer
1  
LOC are a fine tool to use for the one area where they provide a relevant measure - very large projects where they can be used to help give estimates to how long a similar project might take to be completed. Beyond that, people tend to worry about them too much. –  rob Feb 6 '12 at 4:03
    
Right. It is not like LOC does not depe3nd soemtimes on how expressive I write the code, on hformatting rquirements etc. LOC are totally unsuitable and somethin that MBA's witout any experience use. Only. You are free to put yourself into the list of people not understanding why LOC are a bad measurement, but clearly that will not make you look as someone to listen to. –  TomTom Feb 6 '12 at 5:05
    
Please review what I said again, I noted that LOC are a good tool only for one area of measurement and use (i.e. extremely large projects where they can be used for estimation). Anything smaller than the large scale and they use most if not all value for anything beyond quick sound bites in meeting to keep people happy. They are kind of like trying to use light years to measure how fair the coffee shop is from the office, sure you can do it, but the measurement is useless. But when you need to discuss distances between stars they work great. –  rob Feb 6 '12 at 12:29
1  
+1 a function should have all the code it needs to perform the function. It should do one thing and one thing only -- but if that takes a 1000 lines of code then so be it. –  James Anderson Aug 21 '13 at 9:38
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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