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.

Have you ever broken up a large function into smaller functions knowing that those smaller functions will not be called by more than one caller? The primary purpose of a function is to promote code re-use by multiple callers but sometimes I use it to merely organize amd communicate logic.

Do you do the same? Do you think this an acceptable/wrong approach? What are your thoughts.

share

migration rejected from codereview.stackexchange.com Oct 14 '13 at 14:09

This question came from our site for peer programmer code reviews. Votes, comments, and answers are locked due to the question being closed here, but it may be eligible for editing and reopening on the site where it originated.

closed as primarily opinion-based by Jim G., MichaelT, Ozz, Kilian Foth, Dan Pichelman Oct 14 '13 at 14:09

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise.If this question can be reworded to fit the rules in the help center, please edit the question.

    
Great question. But you should really ask this on Programmers where you will get many more reposes and thus a more balanced opinion. –  Loki Astari Aug 24 '12 at 13:26
    
It would be a fatastic exercise to take a large codebase and inline every function that's only used once and then try to understand the code. –  Buhb Aug 24 '12 at 14:21
    
    
possible duplicate of One-line functions that are called only once –  Kilian Foth Oct 14 '13 at 11:37

5 Answers 5

I have seen the break-up-big-functions mentality taken to evil extremes. About half of the worst code I have ever seen involved lots of little single-use procedures. The other half uses the monolith approach. It's hard to say which is worse.

The solution to both problems is the same: Make a clear model of what you are trying to achieve and structure your code according to that model. The quality of your code has almost nothing to do with the size of your procedures, but everything to do with how you choose to slice your project into sections with related functionality and insulate those sections from each other.

Since everyone else has jumped in favor of small procedures, I'll list a few downfalls of arbitrarily small procedures:

  • They can hide related code in little separate fragments.

  • When you make a change inside a procedure, there is a sense that if the changed procedure still makes sense, you are good. If this is a single-use procedure with closely related code scattered among other single-use procedures, this fragmentation discourages the programmer from predicting the side-effects of his or her change.

  • Over time, code ends up being repeated in each fragment, especially initializations, checks for null, and (God help me) database queries.

  • Single-use procedures introduce unnecessary juggling of variables into parameters and assigning return types back to variables. This "bus station" anti-pattern takes extra work to program, to read, and to execute. Sometimes this approach introduces unnecessary data structures to hold the complicated return types of the arbitrary divisions of your larger procedure.

  • One alternative to complicated return types is to create procedures that are used primarily for their side-effects. This is the opposite of functional programming. If you have a database session or stream that you close and maybe reopen in any of the one-use procedures, you are setting yourself up for several different kinds of hard-to-find bugs.

  • Another alternative to complicated return types is to pass mutable objects with all the complication and synchronization nightmares that can cause.

  • DRY: Don't Repeat Yourself. If you avoid cut-and-paste code, most procedures will be self-limiting in size anyway. The code will naturally break itself up into the most useful chunks.

By all means, use procedures to break your code into meaningful chunks of related logic. But flippantly breaking up large procedures into small ones generally only increases complexity. It is not a good pattern to apply thoughtlessly.

True Functions

The word "Function" is often tossed around to mean "Procedure" or "Method" which could involve a database call or other impure side-effects. A true function has no side effects. Meaning that it doesn't perform I/O, throw exceptions, or modify its inputs. Also, a true function can be memoized because if you call it with the same input parameters, it will always return the same output.

Adding impure procedures to your code may sometimes be necessary to avoid cut-and-paste code in fairly performance-critical situations where no history is needed. But this is extremely rare. I would think very carefully before adding a procedure with side-effects to your code unless it keeps you from cutting and pasting.

Like adding a procedure, adding a true function creates a layer of indirection to your code, but if done well it can also create a useful abstraction.

Other Considerations:

If you are working on the front-end, or writing tests, or otherwise working on "leaf node" code that has no other code dependent on it, then monoliths may be the way to go. Avoiding duplication is the only reason to make procedures. I'm particularly thinking that a procedure that only writes to a file or output stream will usually work best as a single unit.

But if you are working on something more upstream, particularly in some kind of middleware that is at a higher level of abstraction, then a procedure of more than about 40 lines usually indicates a design issue. If you are tempted to duplicate something that is used in multiple front-end pieces, break that code off and make middleware out of it!

share

I agree with the other commenters and would add another reason. If you break a large function into smaller ones, it becomes easier to unit-test those small chunks of code, and also more likely that you will remember to do so.

share

Computers don't care if code is in functions or not. That makes it easier for humans to understand the code. Assuming you're a human, that applies just as much for your personal review before the first time you check in code you've just written, as to the first time someone else reads your code.

The issue at hand is humans can only reliably hold around 7 "things" in their head at once (the famous reason for 7-digit phone numbers), and really are a lot more comfortable with only 3 or 4. Functions abstract away things so you can hold the more important parts in your head all at once. In other words, if your function is longer than around 7 "things," it is physiologically difficult for your brain to determine if you got it right. That's reason enough for me to split a function up.

share
1  
Actually, a function call takes time. "Inlining" or expanding function calls by cutting and pasting code, is an optimization technique. It should be the last optimization technique which is used in the most-called code of the innermost loop of a program, and then only when it is absolutely required. It can work against you if the code no longer fits in the cache. Really, I haven't done this in about 20 years and it could easily do more harm than good. But, computers do care, just a little bit whether code is in functions or not. –  GlenPeterson Oct 13 '13 at 19:49
    
@GlenPeterson - couldn't a compiler merge the functions into one procedural flow providing there were no conditions surrounding the function calls? –  Jimmy Breck-McKye Oct 13 '13 at 20:25
1  
@JimmyBreck-McKye You are correct. Inlining methods is a standard compiler optmization. For example, the just in time compiler in the JVM does just this kind inlining. You can find a good write up here -techblug.wordpress.com/2013/08/19/java-jit-compiler-inlining –  DemetriKots Oct 13 '13 at 22:06
    
@DemetriKots - thanks for the link. Interesting. –  Jimmy Breck-McKye Oct 13 '13 at 22:20

There are many reasons to decompose a function into others.

Readability

Which is more readable?

 def optimize(quality, rnd, popsize=1000, n_iters):
   population = [rnd () for _ in xrange (0, popsize)]
   best = None
   for _ in xrange (0, n_iters):
     best = # 100 lines of code to calculate the best quality
     # 100 lines of code to do crossover
     # 100 lines of code to mutate individuals
   return best

vs

 def optimize(quality, rnd, popsize, n_iters):
   population = make_population(pop_size, rnd)
   best = None
   for _ in xrange (0, n_iters):
     best, fitnesses = compute_fitness(population, quality)
     population = mutate(
         crossover(population, rnd), rnd)
   return best

 def compute_fitness(population, quality):
   # 100 lines of code with its own local variables

 def crossover(population, quality):
   # 100 lines of code with its own local variables

 def mutate(population, quality):
   # 100 lines of code with its own local variables

The first gives you the big picture view and allows a reader to decide whether and when to dive into the helper functions. If they were inlined that would not be the case.

This is even before you worry about documenting things like contracts. Most structured documentation schemes allow structured documentation of functions, but not of methods or local variables.

Maintainability

If one function has all its single-use dependencies inlined, then a maintainer has to potentially understand all of the local variables before they make any changes. If the function were decomposed, and the maintainer determined that a bug was in one of the helper functions, then they only need to understand the helper function in detail. Which would you rather have to debug?

def longfunction(a, b, c, d, e, f):
  # 500 lines of code.  Bug manifests somewhere in here.

or

def afunction(a, b, c, d, e, f):
  helper1(a, b, c)
  # Bug manifests because c is now invalid.
  while helper2(c, d, e):
    g = helper3(e, f)
  h = helper4(a, g)
  return helper5(h)

def helper1(a, b, c): # 100 lines of code
def helper2(a, b, c): # 100 lines of code
def helper3(a, b, c): # 100 lines of code
def helper4(a, b, c): # 100 lines of code
def helper5(a, b, c): # 100 lines of code

In the second case, a maintainer stepping through can quickly narrow their search to helper1 and only has to understand in detail 100 lines before coming up with a strategy to fix the bug.

Security

Security is all about isolation -- code is robust against security vulnerabilities when the ability to cause one bit of code to exercise authority in an unintended way does not cause the larger system to exercise authority in an unintended way.

In a language with proper information hiding, you can get security benefits by keeping sensitive objects away from certain code. Object capability languages are designed to make this easy, and often the function is the major means of decomposition.

For example, in an object capability system, if one function delegates a file-system operation to one function and a network operation to another function,

void complexOperation(FileSubtree filetree, Network network, T x) {
  helper1(filetree, x);
  helper2(network, x);
}

Since no complex code ever gets both the filetree and network, it is much easier to reason about what an attacker has to do to cause files to leak over the network. Since x is the only shared input to the two heplers, if x is immutable before helper1, then it cannot have any state from the file tree that might end up being sent over the network.

Although most languages do not have such fine separation of authority, proper decomposition can make it easier to avoid breaches in mainstream programming languages. With small functions, maybe looking at the call graph can convince you that 90% of the LOC of the system never touch the file-system which means a security auditor can focus on the sensitive 10%. With larger less-granular functions the security auditor will necessarily have to consider more of the program.

share

Yes, that is often a good idea. According to Code Complete, there are 16 (yes, 16) valid reasons to create a routine (function, method):

  • Reducing complexity
  • Avoiding duplicate code
  • Limiting effects of changes
  • Hiding sequences
  • Improving performance
  • Making central points of control
  • Hiding data structures
  • Hiding global data
  • Hiding pointer operations
  • Promoting code reuse
  • Planning for a family of programs
  • Making a section of code readable
  • Improving portability
  • Isolating complex operations
  • Isolating use of nonstandard language functions
  • Simplifying complicated boolean tests

Code Complete is an absolutely awesome book, and I consider it a requirement for serious software engineers.

share
1  
+1 for referencing Code Complete. Hearty Dittos to "... absolutely awesome ... a requirement...". And the exhaustive list makes one think - for example at first glance I'd have said "simplifying boolean tests" and "Isolating complex operations" were the same thing. They're not. –  radarbob Aug 24 '12 at 15:00

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