How many lines of code should a function/procedure/method have? [duplicate]

Question

Possible Duplicate:
When is a function too long?

I've recently been given the unenviable task of reviewing poor code written by another developer and documenting the bad practices. (This is all for the purposes of getting out of paying for the developer's work rather than any altruistic reason, of course!)

The reviewed code has several procedures that are many lines of code - the longest is almost 600 lines. A couple of problems with this that I've thought of are maintainability and readability.

The trick is that I need to justify to a layperson why this is a bad practice and if possible back it up with a well regarded and current reference book. Analogies are good too.

Any ideas?

Duplicate: When is a function too long?

Freezedrying

Also check out Cyclomatic Complexity – Chaker

#611088 – Fricke

at leaest one. more, if necessary. – Milstone

Does the code do what it is supposed to do? If so, then chalk this up as a learning experience that oversight should have taken place much sooner than code delivery time. If not, then that is how to get out of paying. I think your current tactic of claiming bad-style is lame. – Genius

@Genius I see your point, however the issue is working with the code after delivery. – Freezedrying

@Alex - I understand that it sucks and I feel for you, but I'm just pointing out that the time to ensure quality is not after the fact. Besides, there is no common agreement on what makes good code, so proving that the code is bad in court (assuming the code works) will be quite hard. – Genius

In the future I would recommend that you review your contractors work from its' design, to the code and even the tests. I would also recommend you defining a coding standards document and require your contractors to code to the standard that you desire. – Genius

And don't accept delivery of code that doesn't meet the minimum standards set in the contract. Once you accept the code, you are pretty much out of luck as it is your responsibility as the contractee to ensure you received what you paid for prior to accepting the product. – Genius

@Genius - Thanks that's great advice. Unfortunately the work had almost been completed by the time I started I started at the company. There has definitely been mismangement of the external developer. – Freezedrying

Exibit A: 1000 lines in System.Windows.Forms.DataGridView.GetClipboardContent(), but there are probably longer ones – Haemolysin

@Haemolysin maybe not really a good "best practice" – Workday

Answer 1

It's not about lines of code. As Steve Mcconnell and Bob Martin say (two pretty good references on coding best practices), a method should do one thing and only one thing. However many lines of code it takes to do that one thing is how many lines it should have. If that "one thing" can be broken into smaller things, each of those should have a method.

Good clues your method is doing more than one thing:

• More than one level of indention in a method (indicates too many logic branches to only be doing one thing)
• "Paragraph Breaks" - whitespace between logical groups of code indicate the method is doing more than one thing

Just to name a few. Bob Martin also says to keep it around 10. Personally I usually try to shoot for 10. If it starts getting close to 20, that's a mental flag to pay closer attention to that method. But ultimately, lines of code are a bad metric for pretty much anything. It is only a helpful indicator that can potentially point to the real issue.

Ningsia

but what if my method has 200 lines of code (it's not designer generated. it's pure business logic)? do you still think, I shouldn't measure by no.of lines of code? – Zip

@SunilBuddala the answer says "a method should do one thing and only one thing. However many lines of code it takes to do that one thing is how many lines it should have." – Ningsia

10 or 20 lines is hard, and like 80 chars per line, doesn't really have benefit, but it establishes a benchmark in double digits. I've got a module that has a 20 - 30 line methods, including a 23 line method, with 4 comment lines including TODOs, and a match statement to reduce lines, but it has some 40-50 line methods that basically just call the other methods and issue reporting statements. Sometimes one reporting statement may consume 3 lines including text to fit line length limitations. So I think it should be said it's also about complexity and how the logic naturally fits together. – Gagman

Answer 2

The Real Answer

There's no specific number.

A Concrete Answer

If you have to justify with some number to lawyers or something, figure out the maximum number of lines that fit on a typical development editor window at your shop, and use that.

General Practice

You shouldn't even really look at it that way, but there should be nothing very complex going on in any one function.

Each unit of work should be delegated to its own unit testable descriptively named method. Do this and all your methods end up tiny and readable without ever counting lines......

The biggest offender I see is 3-4+ boolean conditions exploded in the middle of an if-statement. Wrap all that up in one boolean with a good name, then wrap up any pieces that make it up that are complex in their own.

Frog

In addition, if you're coding in an optimized compiler language (like C or C++) most modern compilers will be able to inline anything that's suitable. So, breaking things out into multiple smaller functions isn't going to induce a lot of call overhead. It also helps to document what's going on if you use intelligent names for the sub-functions. – Chiapas

My personal view from 35+ years of development experience in many languages: It's best to make things simple per screen. Functions longer than 100 lines are sometimes required due to their logic needs, but rarely. If you're in assembly, different story. Also, adding many comments can be beneficial in some cases, and very detracting in others. Like function size, it has to be appropriate for what you're doing. Most functions can be broken out to less than 50 lines of code each. And many of those to 25 lines or less. Keep it as simple as possible, but also logically arranged. – Chiapas

Answer 3

First of all, note that the length restriction is entirely separate from the usual metric, which is "does the function do only one thing, and do it well?" If the answer to that question isn't yes, the function is probably not a good one anyway, regardless of length.

Relevant specifically to the maximum length, a quote from Code Complete, generally considered to be one of the best books on the subject of coding practices:

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. (A line is a noncomment, nonblank line of source code.) Decades of evidence say that routines of such length are no more error prone than shorter routines. Let issues such as depth of nesting, number of variables, and other complexity-related considerations dictate the length of the routine rather than imposing a length restriction per se.

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.

Khan

Answer 4

To add to Rex's point, it should also be as short as possible. Bob Martin says 10 or fewer

Object Mentor - How big should a function be?

Netti

Actually a function should be 5 lines or less with only 79 characters per line and plenty of meaningful descriptive comments and good use of indentation and vertical space :-) – Gagman

Answer 5

As few as possible.

Slagle

I would often rather have sequential code in close proximity (same function) than to break it out and have to scroll around and try to remember what every function does – Clincher

@Clincher That's what I like about inner functions in Python. – Rebecarebecca

It should be fairly obvious what every function does and they should be named so they are easily recognisable. – Gagman

I once had to work with some Python that was constantly using inner functions inside immensely long methods (100's and 100's of lines, sometimes exceeding 1,000). The inner functions seemed to have been employed in order to avoid passing parameters, and were usually called 2 or 3 times in the method. They were diabolical to follow. I started employing vertical highlighting in the code to be able to trace indentation levels which got up to 5 or 6. I've been allergic to inner functions every since. I don't know of any good use case for an inner function other than the standard use of a lambda. – Gagman

Answer 6

It's been many years since I read this, but I think it was in Learning Perl that they recommend making a procedure no longer than you can fit the whole thing on the screen at once. I thought this was a good yardstick. I've seen longer functions that were still readable because of repetitive code (e.g. database access and assigning property values), but those are the exception rather than the norm.

Guitar