7

I'm new at writing professional code (the bulk of my experience is with personal projects) so excuse me if this is trivial. When I write code I find myself being a little inconsistent with how much functionality I include in one function. If I know it's code I'm going to reuse later I'll make a separate function, but when it's something like a check I often find myself either writing a one line function or writing the one line check in every function that needs it.

How little functionality is too little? Is return foo == null; too small and could be included in a bigger function as an if statement, or is it acceptable to write a separate function for just a null check?

8

9 Answers 9

36

Make them as small as possible, but no smaller. The limiting factor here has nothing to do with the work being done. It has to do with if you can think of a good name. A decent vocabulary is a powerful thing.

Give me a choice between a flat 100 line monstrosity and a 10 line function full of functions named procedureA(), procedureB(), and I'll take the monstrosity without even looking.

Give me good names, that abstract a single idea, that don't surprise me when I look inside, and I'll happily choose the little functions.

If you can't think of a good name then please don't create a secret language that only the painfully initiated can speak. I've been hazed enough for one lifetime.

14
  • 1
    Has nothing to do with work being done? What about control of scope and control of flow?
    – John Wu
    Commented Jun 5, 2017 at 20:15
  • 2
    When you keep methods small scoping is a non issue. If you don't keep them small nameless scopes are better than abhorrently named methods. Flow control, if you don't use polymorphism to make it go away, should be made simple and flat even if you aren't messing with tiny methods. Structure and abstraction are simply not the same thing. I can structure code correctly without even thinking up new names. Without a good name good abstraction eludes me every time. Commented Jun 5, 2017 at 20:32
  • You're missing my point. Scoping is an issue that may prevent you from keeping methods small, so saying that size "has nothing to do with" what is going on in the function is too strong a statement, and probably misleading. As for naming, if we are comparing apples to applies, I'd rather have a few badly named small functions than one badly named large function, because I can parse meaning out of the code more easily when the logic is well-structured.
    – John Wu
    Commented Jun 5, 2017 at 20:53
  • 1
    "As small as possible" could just mean adding more and more functions instead of more lines of code.... it is possible to add functions that simply call another function. There is no reason for this. Commented Jun 6, 2017 at 1:08
  • 1
    @FrankHileman I thought something similar. How about creating functions like MOV, ADD, SHL ... hey .. wait...: That indeed does not make sense.
    – Marco13
    Commented Jun 6, 2017 at 1:16
15

Can a function be too short?

Not really.... functions can be as short as you want, even as small as one line. Decent compilers will inline the code if it makes sense to do so, so don't worry that it will add overhead.

Should a simple logical operation be encapsulated in a method?

As for a single line null check-- that might make sense if the null check is on a private variable, or if you consider the knowledge of nullability to be internal, e.g. the caller wouldn't normally think to do the null check himself because it is an implementation detail and not an intuitive aspect of the class's interface.

bool IsDataValid {
    return _privateValue != null;
}

But if the null check would also make sense to the caller, it might make better sense to expect the caller itself to perform the null check against one of the class' fields.

myClass = Factory.GetMyClass();
if (myClass.PublicField != null) {
    //Do Something
}

It really depends on what results in the most readable code.

What are some guidelines on size?

  1. A function should be as short as possible.

  2. A function may grow if it needs to enforce sequential coupling rules, e.g. if there are several steps that need to happen in a particular order. On the other hand, the individual steps could be farmed out to separate functions if the function gets too big.

  3. A function may grow if all of its steps require access to a common but temporary set of local variables, and it would be cumbersome to propagate those variables by including them as input parameters to separate, smaller functions. On the other hand, if there are a ton of variables and the function gets very large and complicated, you could move the function to its own class and promote the local variables to member variables, using the Command design pattern.

  4. A function may grow if consolidation into a single operation makes for a cleaner interface for the caller, e.g. a business level function that wraps an atomic transaction, or a function that represents a unit of work. Again, that doesn't mean the function can't farm out logical steps to various other functions (which might be scoped private so they can't be called directly from outside the class).

  5. In general, a function or method should be readable without scrolling your code pane, if possible.

  6. Developers should strive to minimize cyclomatic complexity in their functions, e.g. minimize the number of possible logic paths. Minimizing complexity has been shown empirically to reduce coder error and defect rate.

2
  • 4
    A function should be as short as possible I really think this is good advice when given to senior level people, but when giving this to people who are more junior this results in a ton of 1-2 line function calls and makes code even more difficult to read. Readability is an important factor for writing code and splitting literally every operation into its own function is a huge source of headache in understanding how code actually works.
    – enderland
    Commented Jun 5, 2017 at 19:55
  • 2
    For me, when working with junior programmers, functions that are too short have never been much of a problem. It is much more common to have functions that are too long.
    – John Wu
    Commented Jun 5, 2017 at 20:01
9

CandiedOrange's answer is closest to the real point. You should not be counting lines to decide if a chunk of code should be encapsulated into a function. Instead, what you want to do is think of your API(s) as a language(s). Now you may (typically will) have multiple layers of APIs corresponding to multiple levels of abstraction. The ideal is to have any block of code operating at a single level of abtraction, i.e. written in a single "language". What you want to avoid is code that looks like this:

void ProcessPayment(User payer, Invoice invoice) {
    if(payer.Ranking > 250) {
        invoice.Balance *= 0.95;
    }
    payer.Points += Math.Floor(invoice.Balance * 0.01);
    this.db.Insert("TransactionTable", new Row(new [] { payer.Id, ... });
    SendThankYouMail(user, invoice);
}

Instead, something like the following is better (though maybe still not ideal):

void ProcessPayment(User payer, Invoice invoice) {
    if(payer.IsPremiumCustomer) {
        invoice.ApplyPremiumCustomerDiscount();
    }
    payer.UpdateRewardPoints(invoice);
    RecordTransaction(payer, invoice);
    SendThankYouMail(payer, invoice);
}

Each of the methods I introduced may only correspond to the one line they replace, but the point is that line was written at a different level of abstraction compared to the rest of the code, and those lines corresponded to the implementation of a concept at a higher level of abstraction. (As a side note, whether the chunk of code was used more than once was also not a factor, though it can be an indicator that there's a concept worth naming.) The second version works more at a single level of abstraction.

But it's not just about naming, though, clearly that is an important facet of designing a language. Even if the names I introduced were good, you could easily argue that the methods called are still not in the same language. Maybe something like this would be better:

void ProcessPayment(User payer, Invoice invoice) {
    ApplyBusinessRules(paymentRules, payer, invoice);
    RecordTransaction(payer, invoice);
    SendThankYouMail(payer, invoice);
}

So, a chunk of code is "too small" to be a function if it only covers part of a (primitive) concept of some higher level abstraction. Difficulty finding a good name is an indicator of this. The problem with large functions, on the other hand, is that that indicates the large function is doing too many tasks at once and/or it is written at too low a level of abstraction and some intermediate level of abstraction should be introduced.

To be clear, a function does not need to be an implementation of a higher level concept in terms of lower level concepts. It can be at the same level and indicate a compound concept, just like the English word "clammy" means "damp and unpleasant". Primitive concepts of a "language" should be encapsulated into functions/objects/whatever and will be instances of implementing a higher level concept in terms of lower level concepts. Such primitive concepts should be encapsulated regardless of how often they are used. Compound concepts built in terms of (but at the same level of abstraction as) primitive concepts are more for convenience and are where looking for duplication makes sense. If a compound concept is used only once, then it probably isn't worth naming.

I've been talking about "layers" of languages and "levels" of abstraction, but it is not quite as simple as some strict hierarchy. For example, the same rule language could be used to specify compliance rules for contracts and validation rules for forms. Designing or seeing these languages is definitely a skill that takes time and effort to develop. A very common problem is while designers may produce a clear notion of nouns and verbs, they don't have a clear (or any) notion of conjunctions. One exercise to help think this way is to imagine what the perfect programming language would be for solving some problem. Of course, more experience with programming, mathematical, and domain languages can broaden your imagination. Capturing the language of the domain experts that will be using/configuring the system is usually a useful thing to do for the higher level languages, and this is fairly widely recognized. Unfortunately, there are other "languages" involved in either implementing that language or implementing the system within which that language will be embedded. So while becoming familiar with the domain experts' language is highly valuable, single-mindedly focusing on it is not.

1
  • Just to let you know: You have a type in your first code snippet. You are using user instead of payer for SendThankYouMail ;)
    – Naxos84
    Commented May 25, 2020 at 11:30
4

Many people focus on the number of lines in a function. This, unfortunately, is not the best measure of how big functions should be.

A function should be coherent, like a laser beam. You should feel like all of the lines in your function are functioning together to accomplish a goal. If you start to feel groups of lines breaking off to form their own coherent thought, that's a good indicator that you should create a function to support them.

A function like return foo == null is clearly coherent. However, you ask if functions can be too small. And the answer to this is "not directly," but indirectly small functions can be a problem. If you have a bunch of small functions, they can reduce the coherency of the functions that call them, by breaking up the task too far. They can force you to constantly look away from the function you're interested in to go read some little one line function to understand what is going on.

So how small is too small? It depends on the context. How easy is it to read code which uses your function. If it's name is descriptive and it's use is intuitive, size is almost irrelevant. Consider the API for shared_ptr. Many of its functions, such as get() and reset() are indeed one-line implementations. However the meaning behind those functions is so well known that you don't lose readability at all. In fact, you increase readability because you encapsulate the data better. It helps you use shared_ptr without losing the coherency of the idea you're putting down in code.

2

For your particular case I'm going to provide an unusual answer and suggest erring on the side of bigger and meatier, but not too big, functions unless you're working in a functional paradigm where things get quite different.

It's because you already seem to have the instincts to refactor and stamp out duplication. You're not the type to write a 300-line function and fail to think twice about it. In those cases, with your instincts in the right place, your tendency will be towards modeling thoughts at too granular of a level to the point where they lose a lot of the richness of meaning and clarity.

Simplicity Can Add Complexity

When you have too many granular functions and classes and so forth in your system modeling the teeniest of human thoughts, it can be even more difficult to comprehend than one with moderately-sized classes and functions. It's because there can often be more intellectual overhead to understanding the interaction between functions and objects than just understanding what they do on their own. One big function, in that sense, can be more difficult to comprehend on its own, but potentially easier to understand as a whole. Meanwhile tiny little functions all calling each other but fulfilling the same purpose as the former bigger (but not too big) function with a complex graph of interdependencies can be much harder to understand when you zoom out and try to figure out everything that's happening for a meaningful operation, since the reader can't just sit down with one function and figure out the big picture. They instead have to end up wading through many functions back and forth trying to keep track of all the state changes along the way, and that's much harder than comprehending a moderately-sized singular function that models a clear human thought.

As a simple example, a 2D video game interacting with granular, object-oriented Pixel objects isn't necessarily easier to comprehend than one interacting with coarser Image or Sprite objects, even though the latter case will have bigger functions that do more things inside than the Pixel version. Pixel is too granular of an idea to model what the bulk of what a video game is interested in doing.

Even as an implementation detail of Images or Sprites, having the logic separated and dispersed between a teeny Pixel object and bigger Image object could actually make the code more difficult to comprehend at the level of the image operation being performed, especially if that introduces interactions from Image->Pixel, Pixel->Pixel, and Pixel->Image. In such a case, it can often improve the maintainability and the ability to comprehend such code if you demolish the Pixel interface and turn the idea of a Pixel into raw data, not a teeny object with teeny functions, even if that means adding slightly more boilerplate to each image function.

You can potentially make more gains in maintainability and clarity and the ability to reason about what's going on in your code at a meaningful level by seeking to reduce the number of interactions that go on between functions and objects and flatten and centralize the meaningful operations in your system instead of scattering and fragmenting that logic all over the place. That can potentially get you further than trying to dice up the objects and functions into smaller and smaller pieces if the exchange requires introducing a spiderweb of interactions in your system.

Flattening the System

I've found a whole lot of practical benefits for reasons that are hard to perfectly explain by "flattening" my codebases. As a very simple example, consider a video game which loops through the entities in the game world, applies physics, plays any audio, animates characters with bones and inverse kinematics, etc, and renders the final result, and all in one single pass.

Meanwhile, instead let's say you have a PhysicsSystem, an AnimationSystem, an AudioSystem, and RenderSystem. Each one loops through all the entities in the game world in their own separate passes, with the physics pass doing nothing more than applying physics, instead of doing all of this in one pass like we did above. The systems don't communicate with each other at all. They just access the list of entities in the game world to process without caring about what each other are doing, and all the entities in the world just consist of raw data for their components (they have no functionality of their own).

While that can result in more passes and loops and bigger functions in the central systems, now each system is performing a singular responsibility. However, SRP isn't necessarily the primary benefit. The above model doing everything in one pass might still clearly separate physics functions from animation functions in their separate packages, e.g. The big difference here is that each system is completely isolated in its own world with bulky logic associated with each. When the systems are applying a "batch job" applying to all entities relevant in the game world instead of one entity at a time, that tends to result in slightly more code in the central systems but fewer code interactions to reason about.

Just in practice, I find this "flatter" system with bulkier "objects" (PhysicsSystem, not Particle object) so much easier to reason about correctness and comprehend at a meaningful level closer to what users actually do with the engine and mainly because of the fewer interactions between functions. However, it does generally imply at least slightly bigger functions than if you were to handle, say, physics on a per-entity basis in functions that just apply to one little thing at a time. The nature of the control flow easily allows you to "zoom out" and comprehend everything as a whole instead of tempting you to "zoom in" and try to figure out every little thing.

Granularity Losing Meaning

An extreme example is to start writing "helper" functions, named as such, simply to reduce a couple of lines of code repeated 3 times (6 LOC) to 3 lines of code calling a "helper" function which can't really be documented very well beyond the fact that it slightly reduces the code required to implement functions A and B. In that case the "helper" functions are about as intuitively designed as what a machine might generate for a "code reducer" software which would only reduce the maintainability of the code. In that case, I'd even suggest just writing a bit more code even if it's "boilerplatey" if you can't reduce the amount in a clear and meaningful way.

Similar to CandiedOrange's answer, your functions should model a clear idea with an obvious purpose, easily documented. If you're working at too granular of a level and too entrenched in the code that you're writing functions so granular that you can't do this effectively anymore, that's when I'd suggest to err on the side of bigger functions.

Balancing Act

Of course some clear ideas with obvious purposes just map directly to a line of code. A function to retrieve the number of bytes remaining from a file stream probably doesn't need more than a line or two of code, but represents very clear functionality and one which will frequently be desired by those who use the code. Those are fine. Some functions need to be very short, but not all need to be. It's not always the case that shorter is better. It's a balancing act.

So when does the function get too big? That's tougher to answer and will be something you hone with experience. But instead of focusing on the number of lines of code, some basic things to look at:

  1. Side effects. Does the function cause changes to states outside of its scope, like modifying the parameters being passed to it or mutating the data members in a class? If so, a function that causes many logical side effects is often very difficult to comprehend as well as error-prone if it ever needs changes. So try to reduce the logical side effects to one max or ideally zero.

  2. How many local variables does the function need to manage? This isn't as bad as side effects, but a function that needs, say, a dozen local variables visible to many lines of code in the function is also difficult to comprehend.

  3. Does the function perform a clear, singular responsibility? This question is related to human thought processes. A function to DestroyWindow might need to do many things computationally but still models a very clear human thought and only causes one logical side effect.

  4. How many levels of indentation do you need (assuming indents are used for blocks)? Linus Torvalds once said something along the lines that if your function requires more than 3 levels of indentation, it's broken and needs to be fixed. While I don't believe in putting hard numbers like this, a function that needs a lot of nested blocks and scopes often requires you to push and pop to/from a complex mental stack in your brain as you trace through it. If you find yourself reaching for many levels of indentation, that might be a time to consider refactoring.

0

My shortest functions (often during development) sometimes just return a constant. And this works like this: when i encounter e.g. a filename which is used at different occasions, for thich i know that there will be a programmatic way of calculating it later (e.g. after the precise way of installing and maintaining the program is settled), but that right now (e.g. in the Minimum Viable Product, this Sprint, this Release, this branch) this is not going to happen, then i will create a function which right now returns a constant but will do something more complex in the future.

So instead of asking "how short can it be", you should ask "is this a separate aspect, reused at many places, which is likely to gain an indenpendent nontrivial behaviour". A typical separation would be: decouple functions which prepare/adapt to environment from computational functions.

1
  • I frequently create such one-line functions too. I also add a "todo" or "note to future self" comment about possible improvements. Commented Jun 12, 2017 at 3:02
0

Have you seen the Clean Coders series by Uncle Bob? He actually covers over this question and mentions functions should really not be over 4 lines. He goes on to state that functions should be as small as possible, and should only really be working to achieve one thing.

While i don't necessarily think they should be 4 lines (from experience i find 4-6 works best as opposed to a strict 4), keeping them as small as possible while also keeping functions focused on doing one task should be what you aim for.

Also as a bit of a tangent, make sure you pick well structured and worded function names, it helps to document everything and makes it easier for future programmers to read (of course if that's your intention).

0

As a short and readable answer, there are 2 things to keep in mind:

  1. It is not about the size, it is how you use it.
  2. It is also about what could be done to help the code being more readable by breaking down a problem solved by a function into multiple smaller problems solved by smaller functions.

It helps to keep high level concerns inside one function and specific low level concerns into several smaller, dedicated functions.

-3

There is an easy answer in two lines:

  • as small as possible
  • as big as needed

each function should have its very specific concern.

3
  • 20
    I am struggling to see how this answer could possibly help someone who is at a knowledge level where they would have to ask this question to begin with.
    – John Wu
    Commented Jun 5, 2017 at 18:49
  • Ah, so it should be between 10^2 and 10^3 LOC? brb refactoring Commented Jun 6, 2017 at 7:52
  • While i cant argue with the fact that everything youve said is correct, this answer really isnt very helpful to anyone who might be asking this question in the first place... actually, what @JohnWu said.
    – richzilla
    Commented Jun 6, 2017 at 9:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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