61

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?

Dave Jarvis
  • 30,436
  • 41
  • 178
  • 315
Alex Angas
  • 59,219
  • 41
  • 137
  • 210
  • http://stackoverflow.com/questions/610588/when-should-i-break-a-function – Greg Mar 04 '09 at 16:28
  • 3
    at leaest one. more, if necessary. – Steven A. Lowe Mar 04 '09 at 16:32
  • 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. – Dunk Mar 04 '09 at 16:37
  • @Dunk I see your point, however the issue is working with the code after delivery. – Alex Angas Mar 04 '09 at 16:54
  • 1
    @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. – Dunk Mar 04 '09 at 21:49
  • 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. – Dunk Mar 04 '09 at 21:51
  • 1
    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. – Dunk Mar 04 '09 at 21:56
  • @Dunk - 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. – Alex Angas Mar 06 '09 at 12:14
  • Also check out [Cyclomatic Complexity](http://en.wikipedia.org/wiki/Cyclomatic_complexity) – Hawk Kroeger Mar 04 '09 at 16:25
  • Exibit A: 1000 lines in [System.Windows.Forms.DataGridView.GetClipboardContent()](https://referencesource.microsoft.com/#System.Windows.Forms/winforms/Managed/System/WinForms/DataGridViewMethods.cs.html,f532fe762c86f033), but there are probably longer ones – Slai Feb 07 '17 at 13:31
  • @Slai maybe not really a good "best practice" – Rene Mar 17 '21 at 10:18

6 Answers6

153

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.

Stypox
  • 963
  • 11
  • 18
Rex M
  • 142,167
  • 33
  • 283
  • 313
  • 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? – Sunil Buddala Jul 09 '19 at 00:34
  • 3
    @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." – Rex M Jul 09 '19 at 14:32
18

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.

NickC
  • 42
  • 10
Trampas Kirk
  • 1,436
  • 3
  • 16
  • 21
  • 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. – Rick C. Hodgin Apr 28 '23 at 20:25
  • 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. – Rick C. Hodgin Apr 28 '23 at 20:27
11

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.

Community
  • 1
  • 1
Chad Birch
  • 73,098
  • 23
  • 151
  • 149
3

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?

josliber
  • 43,891
  • 12
  • 98
  • 133
Ovi Tisler
  • 6,425
  • 3
  • 38
  • 63
3

As few as possible.

Alex Fort
  • 18,459
  • 5
  • 42
  • 51
  • 1
    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 – MStodd Aug 19 '22 at 16:11
  • 1
    @MStodd That's what I like about inner functions in Python. – hotplasma Jan 10 '23 at 13:28
2

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.

flatline
  • 42,083
  • 4
  • 31
  • 38