Consider a parameterless (edit: not necessarily) function that performs a single line of code, and is called only once in the program (though it is not impossible that it'll be needed again in the future).

It could perform a query, check some values, do something involving regex... anything obscure or "hacky".

The rationale behind this would be to avoid hardly-readable evaluations:

if (getCondition()) {
    // do stuff

where getCondition() is the one-line function.

My question is simply: is this a good practice? It seems alright to me but I don't know about the long term...

  • 9
    Unless your code fragment is from a OOP language with implicit receiver (e.g. this) this certainly would be a bad practice, as getCondition() most probably relies on global state ...
    – Ingo
    Commented Sep 12, 2011 at 14:38
  • 3
    He may have meant to imply that getCondition() could have arguments?
    – user606723
    Commented Sep 12, 2011 at 16:25
  • 25
    @Ingo -- some things really have Global state. Current time, hostnames, port numbers etc. are all valid "Globals". The design error is making a Global out of something that is inherently not global. Commented Sep 13, 2011 at 1:42
  • 1
    Why don't you just inline getCondition? If it's as small and infrequently used as you say it is then giving it a name is not accomplishing anything.
    – user7146
    Commented Sep 13, 2011 at 4:22
  • 12
    davidk01: Code readability.
    – wjl
    Commented Sep 13, 2011 at 6:02

12 Answers 12


Depends on that one line. If the line is readable and concise by itself, the function may not be needed. Simplistic example:

void printNewLine() {

OTOH, if the function gives a good name to a line of code containing e.g. a complex, hard to read expression, it is perfectly justified (to me). Contrived example (broken into multiple lines for readability here):

boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isFemale() 
        && (taxPayer.getNumberOfChildren() > 2 
        || (taxPayer.getAge() > 50 && taxPayer.getEmployer().isNonProfit()));
  • 103
    +1. The magic word here is "Self-documenting code".
    – Konamiman
    Commented Sep 12, 2011 at 15:17
  • 8
    A good example of what Uncle Bob would call "encapsulating conditionals." Commented Sep 12, 2011 at 18:07
  • 13
    @Aditya: Nothing says that taxPayer is global in this scenario. Perhaps this class is TaxReturn, and taxPayer is an attribute. Commented Sep 12, 2011 at 22:14
  • 2
    Additionally it allows you to document the function in a way that can be picked up by e.g. javadoc and be publicly visible.
    – user1249
    Commented Sep 13, 2011 at 7:04
  • 2
    @dallin, Bob Martin's Clean Code shows lots of semi-real life code examples. If you have too many functions in a single class, maybe your class is too big? Commented Sep 6, 2013 at 16:09

Yes, this can be used to satisfy best practices. For instance, it is better to have a clearly-named function do some work, even if it is only one line long, than to have that line of code within a larger function and need a one-line comment explaining what it does. Also, neighbouring lines of code should perform tasks at the same abstraction level. A counterexample would be something like

petrolFlag |= 0x006A;

In this case it is definitely better to move the middle line into a sensibly-named function.

  • 15
    That 0x006A is lovely :D A well known constant of carbonized fuel with additives a, b and c.
    – Coder
    Commented Sep 12, 2011 at 15:33
  • 2
    +1 I'm all for self-documenting code :) by the way, I think I agree on keeping a constant abstraction level through blocks, but I couldn't explain why. Would you mind expanding that?
    – deprecated
    Commented Sep 12, 2011 at 17:18
  • 3
    If you're just saying that petrolFlag |= 0x006A; without any kind of decision making, it would be better to simply say petrolFlag |= A_B_C; without an additional function. Presumably, engageChoke() should only be called if petrolFlag meets a certain criteria and that should clearly say 'I need a function here.' Just a minor nit, this answer is basically spot on otherwise :)
    – user131
    Commented Sep 12, 2011 at 18:56
  • 9
    +1 for pointing out that code within a method should be in the same abstraction level. @vemv, it is good practice because it makes the code easier to understand, by avoiding the need for your mind to jump up and down between different levels of abstraction while reading the code. Tying abstraction level switches to method calls/returns (i.e. structural "jumps") is a nice way to make the code more fluent and clean. Commented Sep 13, 2011 at 6:55
  • 22
    No, it's a completely made-up value. But the fact that you wonder about it just emphasizes the point: if the code had said mixLeadedGasoline(), you wouldn't have to! Commented Sep 13, 2011 at 8:22

I think that in many cases such a function is good style, but you may consider a local boolean variable as alternative in cases when you don't need to use this condition somewhere in other places e.g.:

bool someConditionSatisfied = [complex expression];

This will give a hint to code reader and save from introducing a new function.

  • 1
    The bool is particularly beneficial if the condition function name would be difficult or possibly misleading (e.g. IsEngineReadyUnlessItIsOffOrBusyOrOutOfService).
    – dbkk
    Commented Sep 12, 2011 at 20:01
  • 2
    I experienced this advice a bad idea: the bool and the condition distract attention from the core business of the function. Also, a style preferring local variables make refactoring harder.
    – Wolf
    Commented Jan 9, 2015 at 9:22
  • 1
    @Wolf OTOH, I prefer this to a function call in order to reduce the "abstraction depth" of code. IMO, jumping into a function is a larger context switch than an explicit and direct couple lines of code, especially if it just returns a boolean.
    – Kache
    Commented Oct 19, 2016 at 4:44
  • @Kache I think it depends on whether you code object-oriented or not, in the OOP case, the use of member function keeps the design much more flexible. It really depends on the context...
    – Wolf
    Commented Oct 19, 2016 at 6:03

In addition to Peter's answer, if that condition may need to be updated at some point in the future, by having it encapsulated the way you suggest you would only have a single edit point.

Following Peter's example, if this

boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isFemale() 
        && (taxPayer.getNumberOfChildren() > 2 
        || (taxPayer.getAge() > 50 && taxPayer.getEmployer().isNonProfit()));

becomes this

boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isMutant() 
        && (taxPayer.getNumberOfThumbs() > 2 
        || (taxPayer.getAge() > 123 && taxPayer.getEmployer().isXMan()));

You make a single edit and it's updated universally. Maintainability wise, this is a plus.

Performance wise, most optimizing compilers will remove the function call and inline the small code block anyway. Optimizing something like this can actually shrink the block size (by deleting the instructions needed for the function call, return, etc) so it's normally safe to do even in conditions that might otherwise prevent inlining.

  • 2
    I'm already aware of the benefits of keeping a single edit point - that's why the question says "...called once". Nonetheless it's great to know about those compiler optimizations, I always thought they followed this kind of instructions literally.
    – deprecated
    Commented Sep 12, 2011 at 17:16
  • Splitting out a method to test a simple condition tends to imply that a condition may be changed by changing the method. Such implication is useful when it's true, but can be dangerous when it's not. For example, suppose that code needs to divide things into lightweight objects, heavy green objects, and heavy non-green objects. Objects have a quick "seemsGreen" property which is reliable for heavy objects, but may return false positives with lightweight objects; they also have a "measureSpectralResponse" function which is slow, but will work reliably for all objects.
    – supercat
    Commented Sep 2, 2014 at 19:37
  • The fact that seemsGreen will be unreliable with lightweight objects will be irrelevant if code never cares whether lightweight objects are green. If the definition of "lightweight" changes, however, such that some non-green objects which return true for seemsGreen aren't reported as "lightweight", then such a change to the definition of "lightweight" could break the code that tests for objects being "green". In some cases, testing for green-ness and weight together in the code may make the relationship between the tests clearer than if they're separate methods.
    – supercat
    Commented Sep 2, 2014 at 19:41
  • This is also a nice example where it is absolutely clear what the code does (checks gender, number of children, age, employer) but that doesn't give us a clue about the meaning of the code. The function "getsChildCareVouchers()" might have very, very similar code. Or "nextToBeFired()" at the wrong company.
    – gnasher729
    Commented Oct 27, 2021 at 12:16

In addition to readability (or in complement of it) this allows to write functions at the proper level of abstraction.

  • 2
    I'm afraid I don't understand what you meant...
    – deprecated
    Commented Sep 12, 2011 at 20:28
  • 2
    @vemv: I think he means by "proper level of abstraction" that you don't end up with code that has different levels of abstraction mixed (i.e. what Kilian Foth already said). You don't want your code to handle seemingly insignificant details (e.g. the formation of all bonds in fuel) when the surrounding code is considering a more high-level view of the situation at hand (e.g. running an engine). The former clutters the latter and makes your code less easy to read and maintain as you will have to consider all levels of abstraction at once at any time.
    – Egon
    Commented Sep 12, 2011 at 22:02

It depends. Sometimes it's better to encapsulate an expression in a function/method, even if it's only one line. If it's complicated to read or you need it in multiple places, then I consider it a good practice. In the long term it's easier to maintain, as you've introduced a single point of change and better readability.

However, sometimes it's just something you don't need. When the expression is easy to read anyway and/or just appears in one place, then don't wrap it.


I think if you only have a few of them then it is okay but the issue comes up when there are a lot of them in your code. And when the compiler runs or the interpitor (depending on the language you use) It is going to that function in memory. So lets say you have 3 of them I dont think the computer will notice but if you start having 100's of those little things then the system has to register functions in memory that are only called once and then not destroyed.

  • According to Stephen's answer this might not always happen (although trusting blindly in compilers' magic can't be good anyway)
    – deprecated
    Commented Sep 13, 2011 at 5:59
  • 1
    yeah it should get cleared out depending on a lot of facts. If its an interpated language the system will fall for the single line functions everytime unless you install something for cache which still may not do the trick. Now when it comes to compilers it just matters onn the day of the weak and the placment of the planates if the compiler will be your friend and clear out your little mess or if its going to thinkk you really need it. I remeber that if you know the exact amoutn of times a loop will always run its some times better to just copy and paste it that many times then to loop.
    – WojonsTech
    Commented Sep 13, 2011 at 6:07
  • +1 for alignment of the planets :) but your last sentence sounds totally nuts to me, do you really do that?
    – deprecated
    Commented Sep 13, 2011 at 7:41
  • It really depends Most of the time no i do not unless i am getting the correct amount of payment to check if it does speed incress and other things like that. But in older compilers it was better to copy and paste it then to leave the compilere to figure it out 9/10.
    – WojonsTech
    Commented Sep 14, 2011 at 0:58

I've done this exact thing just recently in an application that I've been refactoring, to make explicit the actual meaning of the code sans comments:

protected void PaymentButton_Click(object sender, EventArgs e)
    Func<bool> HaveError = () => lblCreditCardError.Text == string.Empty && lblDisclaimer.Text == string.Empty;



  • Just curious, what language is that?
    – deprecated
    Commented Sep 12, 2011 at 20:27
  • 5
    @vemv: Looks like C# to me. Commented Sep 12, 2011 at 20:37
  • I also prefer extra identifiers over comments. But does this really differ from introducing a local variable to keep the if short? This local (lambda) approach makes the PaymentButton_Click function (as a whole) harder to read. The lblCreditCardError in your example seems to be a member, so also HaveError is a (private) predicate that is valid for the object. I'd tend to downvote this, but I'm not a C# programmer, so I resist.
    – Wolf
    Commented Jan 9, 2015 at 9:35
  • @Wolf Hey man, yeah. I wrote this quite a while ago :) I definitely do things quite differently now days. In fact, looking at the label content to see if there was an error makes me cringe... Why didn't I just return a bool from CheckInputs()???
    – joshperry
    Commented Jan 9, 2015 at 17:38

Moving that one line into a well named method makes your code easier to read. Many others have already mentioned that ("self-documenting code"). The other advantage of moving it into a method is that it makes it easier to unit test. When it's isolated into it's own method, and unit tested, you can be sure that if/when a bug is found, it won't be in this method.


There are already a lot of good answers, but there is a special case worth mentioning.

If your one-line statement needs a comment, and you are able to clearly identify (which means: name) its purpose, consider extracting a function while enhancing the comment into API doc. This way you make the function call easier faster and to understand.

Interestingly, the same can be done if there is currently nothing to do, but a comment reminding of expansions needed (in the very near future1)), so this,

def sophisticatedHello():
    # todo set up
    # todo tear down

could as well changed into this

def sophisticatedHello():

1) you should be really sure about this (see YAGNI principle)


If the language supports it, I usually use labeled anonymous functions to accomplish this.

someCondition = lambda p: True if [complicated expression involving p] else False
#I explicitly write the function with a ternary to make it clear this is a a predicate
if (someCondition(p)):
    #do stuff...

IMHO this is a good compromise because it gives you the readability benefit of not having the complicated expression cluttering the if condition while avoiding cluttering the global/package namespace with small throw-away labels. It has the added benefit that the function "definition" is right where its being used making it easy to modify and read the definition.

It doesn't have to only be predicate functions. I like to encase repeated boiler-plate in small functions like this as well (It works particularly well for pythonic list generation without cluttering the bracket syntax). For example, the following oversimplified example when working with PIL in python

#goal - I have a list of PIL Image objects and I want them all as grayscale (uint8) numpy arrays
im_2_arr = lambda im: array(im.convert('L')) 
arr_list = [im_2_arr(image) for image in image_list]
  • Why " lambda p: True if [complicated expression involving p] else False " instead of " lambda p: [complicated expression involving p] " ? 8-) Commented Sep 13, 2011 at 6:42
  • @hstoerr its in the comment right below that line. I like to make it explicitly known that we someCondition is a predicate. While its strictly unnecessary, I write a lot of scientific scripts read by people who don't code that much, I personally think its more appropriate to have the extra terseness rather than have my colleagues confused because they don't know that [] == False or some other similar pythonic equivalence that isn't 'always' intuitive. Its basically a way to flag that someCondition is, in fact, a predicate.
    – crasic
    Commented Sep 13, 2011 at 6:56
  • Just to clear my obvious mistake [] != False but [] is False as a when cast to a bool
    – crasic
    Commented Sep 13, 2011 at 7:03
  • @crasic: when I don't expect my readers to know that [] evaluates to False, I prefer doing len([complicated expression producing a list]) == 0, rather than using True if [blah] else False which still requires the reader to know that [] evaluates to False.
    – Lie Ryan
    Commented Sep 13, 2011 at 7:33

Here is a pretty good rule of thumb I use:

Would it be a good (or at least reasonable) idea to add a comment explaining that line? If yes, then refactor to a function with a name that gives the same information as the comment you planned to write.

This approach works perfectly for mathematical code, like

fun distanceSquared(p1, p2) {
    return (p1.x - p2.x)**2 + (p1.y - p2.y)**2;

fun distance(p1, p2) {
    return sqrt(distanceSquared(p1, p2));

You might wonder about the first function. It's actually quite useful sometimes for performance reasons. For instance, if(distance(p1, p2) > distance(p3, p4)) gives the same result if you switch to distanceSquared, but you get to skip two sqrt calculations. So you might want to write this:

fun distanceCompare(p1, p2, p3, p4) {
    return distanceSquared(p1, p2) - distanceSquared(p3, p4);

That will return a negative number if the distance (p1,p2) is smaller than (p3,p4), zero if equal, and a positive number otherwise.


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