Using functions as a way to stylize

Question

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.

Answer 1

The primary purpose of a function is to promote code re-use by multiple callers

not so.

The primary One purpose of a function is to promote code re-use by multiple callers

That's better.

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?

Absolutely, yes.

Why?

• Make the surrounding structure clear
• put it in business domain terms
  • i.e. IsElegibleForFrazination() vice if (A || B && (!C && !D))
• reduce visual clutter, thus enhancing readability
• Maintain the level of abstraction I'm (coding) in (in this particular method).
• Expressability, convey intent
  • DangerWillRobinson() vice some tricky operation in-line.
• Re-useability
  • Oh yeah, there's that. I put it last because I seem to use the others above much more often.

Answer 2

Absolutely acceptable, in fact it is encouraged where I work. Nothing worse than trying to fight your way through a 300 line function trying to sort out logic. The idea to split is apart is to make the main logic easier to follow.

I know this isn't a hard bit to decipher, but to understand the following code, you would have trace through all of the possible combinations of the if statement. The intent is not clear, making it really hard to understand.

if ((shape.IsCircle() && ((Circle)shape).Radius == 1) || ((shape.IsSquare) && ((Square)shape).SideSize == 1)
{
     // Do some complex processing
}

If you pull the if statement out, you can now understand what the intent of the check was for, and decide if that is the code you're looking for. If you need to understand how that check is happening, you can go study the function. If you don't care, you can skip over it, and still have an understanding of what is going on in the calling function.

if (ShapeAllowsForDisplayingAllWidgets(shape))
{
    // Do some complex processing
}

All that being said, you could move the complex processing to its own function, with a clear name, and pass in the result from your first one, eliminating the need of the if statement.

DisplayWidgets(ShapeAllowsForDisplayingAllWidgets(shape));

Reads almost like english, and allows you to find specific logic very easily.

Answer 3

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.

Answer 4

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.

Answer 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 structrues 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 use object variables instead of method variables - with all the synchronization nightmares that can cause.

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.

EDIT: I've been thinking about this for about a month since I wrote it and I think the ideal size of your procedures depends on your domain. 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!

Answer 6

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.

Answer 7

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.