Can a function be too short?

Question

Whenever I find myself writing the same logic more than once, I usually stick it in a function so there is only one place in my application I have to maintain that logic. A side effect is that I sometimes end up with one or two line functions such as:

function conditionMet(){
   return x == condition;
}

OR

function runCallback(callback){
   if($.isFunction(callback))
     callback();
}

Is this lazy or a bad practice? I only ask because this results in a greater number of function calls for very tiny pieces of logic.

Answer 1

Hehe, oh Mr Brown, if only I could persuade all the developers I meet to keep their functions as small as this, believe me, the software world would be a better place!

1) Your code readability increases ten fold.

2) So easy to figure out the process of your code because of the readability.

3) DRY - Don't Repeat Yourself - You're conforming to this very well!

4) Testable. Tiny functions are a million times easier to test than those 200 line methods we see way too often.

Oh and don't worry about "function hopping" in terms of performance. "Release" builds and compiler optimisations take care of this for us very nicely, and performance is 99% of the time somewhere else in the systems design.

Is this lazy? - Very much the opposite!

Is this bad practice? - Absolutely not. Better to pulled this way of making methods than the tar balls or "God Objects" that are oh so too common.

Keep up the good work my fellow craftsman ;)

Answer 2

I would say that a refactored method is too short if either:

• It duplicates a primitive operation, for no other purpose than to make it a method:

Ex:

boolean isNotValue() {
   return !isValue();
}

or...

• The code is only used once, and its intent is easy to understand at a glance.

Ex:

void showDialog() {
    Dialog singleUseDialog = new ModalDialog();
    configureDialog(singleUseDialog);
    singleUseDialog.show();
}

void configureDialog(Dialog singleUseDialog) {
    singleUseDialog.setDimensions(400, 300);
}

This could be a valid pattern, but I would just inline the configureDialog() method, in this example, unless I intended to override it or reuse this code elsewhere.

Answer 3

Can a function be too short? In general no.

In fact the only way to ensure that:

1. You have found all the classes in your design
2. Your functions are doing only one thing.

Is to keep your functions as small as possible. Or, in other words, extract functions from your functions until you can't extract any more. I call this "Extract till you drop."

To explain this: A function is a scope with chunks of functionality that communicate by variables. A class is also a scope with chunks of functionality that communicate by variables. So a long function can always be replaced by one or more classes with small method.

Also, a function that is big enough to allow you to extract another function from it, is doing more than one thing by definition. So if you can extract a function from another, you should extract that function.

Some folks worry that this will lead to a proliferation of functions. They're right. It will. That's actually a good thing. It's good because functions have names. If you are careful about choosing good names, then these functions act as sign posts that direct other people through your code. Indeed, well named functions inside of well named classes inside of well named namespaces are one of the best ways to make sure that your readers do NOT get lost.

There's a lot more about this in Episode III of Clean Code at cleancoders.com

Answer 4

Wow, most of these answers aren't very helpful at all.

No function should be written whose identity is its definition. That is, if the function name is simply the function's code block written out in English, then don't write it as a function.

Consider your function conditionMet and this other function, addOne (forgive me for my rusty JavaScript):

function conditionMet() { return x == condition; }

function addOne(x) { return x + 1; }

conditionMet is a proper conceptual definition; addOne is a tautology. conditionMet is good because you don't know what conditionMet entails just by saying "condition met", but you can see why addOne is silly if you read it out in English:

"For the condition to be met is for x to equal condition" <-- explanatory! meaningful! useful!

"To add one to x is to take x and add one." <-- wtf!

For the love of anything that might still be holy, please, don't write tautological functions!

(And for the same reason, don't write comments for every line of code!)

Answer 5

I'd say that if you think the intention of some code can be improved by adding a comment, then rather than adding that comment, extract the code into its own method. No matter how small the code was.

So for example if your code was going to look like:

if x == 1 { ... } // is pixel on?

make it look like this instead:

if pixelOn() { ... }

with

function pixelOn() { return x == 1; }

Or in other words, it's not about the method length, but about self-documenting code.

Answer 6

I think this is exactly what you want to do. Right now that function might only be one or two lines but over time it could grow. Also having more function calls allows you to read the function calls and understand what is happening inside there. This makes your code very DRY (Don't Repeat Yourself) which is much more maintainable.

Answer 7

I agree with all the other posts I have seen. This is good style.

The overhead of such a small method may be nil as the optimizer may optimize the the call away and inline the code. Simple code like this is allows the optimizer to do its best work.

Code should be written for clarity and simplicity. I try to limit a method to one of two roles: making decisions; or performing work. This may generate one line methods. The better I am at doing this, the better I find my code is.

Code like this tends to have high cohesion and low coupling which is good coding practice.

EDIT: A note on method names. Use a method name which indicates what the method does not how it does it. I find verb_noun(_modifier) is a good naming scheme. This give names like Find_Customer_ByName rather than Select_Customer_Using_NameIdx. The second case is prone to become incorrect when the method is modified. In the first case, you can swap out the entire Customer database implementation.

Answer 8

Refactoring one line of code into a function seems excessive. There might be exceptional cases, such as ver loooooong/comples lines or expessions, but I wouldn't do this unless I know the function will grow in the future.

and your first example hints at use of globals (which may or may not speak of other issues in the code), I'd refactor it further, and make those two variables as parameters:

function conditionMet(x, condition){
   return x == condition;
}
....
conditionMet(1,(3-2));
conditionMet("abc","abc");

The conditionMet example might be useful if the condition was long and repetitive such as:

function conditionMet(x, someObject){
   return x == ((someObject.valA + someObject.valB - 15.4) / /*...whole bunch of other stuff...*/);
}

Answer 9

Consider this:

A simple collision detection function:

bool collide(OBJ a, OBJ b)
{
    return(pow(a.x - b.x, 2) + pow(a.y - b.y, 2) <= pow(a.radius + b.radius, 2));
}

If you wrote that "simple" one liner in your code all the time, you might eventually make a mistake. Plus, it'd be really torturous to write that over and over.

Answer 10

No, and that is rarely a problem. Now if someone feels no function should be longer than one line of code (if only it could be that simple), that would be a problem and in some ways lazy because they are not thinking about what is appropriate.

Answer 11

I would say they are too short, but this is my subjective opinion.

Because:

• There is no reason to create a function if it's used only once or twice. Jumping to defs suck. Especially with amazingly fast VS and C++ code.
• Class overview. When you have thousands of small functions, it drives me angry. I enjoy when I can view class definitions and quickly see what it does, not how it SetXToOne, SetYToVector3, MultiplyNumbers, + 100 setters/getters.
• In most projects these helpers become dead weight after one ore two refactoring phases, and then you do "search all"->delete to get rid of obsolete code, usually ~25%+ of it.

Functions that are long are bad, but functions that are shorter than 3 lines and perform only 1 thing are equally bad IMHO.

So I'd say only write small function if it's:

• 3+ lines of code
• Does stuff that junior devs might miss (not know)
• Does extra validation
• Is used, or will used at least 3x
• Simplifies frequently used interface
• Will not become a dead weight during next refactoring
• Has some special meaning, for example, template specialization or something
• Does some isolation job - const references, affects mutable parameters, does private member retrieval

I bet next developer (senior) will have better things to do than to remember all your SetXToOne functions. So they'll turn into dead weight pretty soon either way.

Answer 12

I don't like the example no. 1, bcause of ith generic name.

conditionMet does not seem to be generic, so it stands for a specific condition? Like

isAdult () = { 
  age >= 18 
}

This would be fine. It's a semantic difference, while

isAtLeast18 () { age >= 18; } 

would not be fine for me.

Maybe it is often used, and can be subject for later change:

getPreferredIcecream () { return List ("banana", "choclate", "vanilla", "walnut") }

is fine too. Using it multiple times, you just need to change a single place, if you have to - maybe whipped cream gets possible tomorrow.

isXYZ (Foo foo) { foo.x > 15 && foo.y < foo.x * 2 }

is not atomic, and should give you a nice test opportunity.

If you need to pass a function, of course, pass whatever you like, and write otherwise silly looking functions.

But in general, I see much more functions, which are too long, than functions which are too short.

A last word: Some functions only look appropriate, because they're written too verbose:

function lessThan (a, b) {
  if (a < b) return true else return false; 
}

If you see, that it is the same as

return (a < b); 

you won't have a problem with

localLessThan = (a < b); 

instead of

localLessThan = lessThan (a, b); 

Answer 13

Too short never is a problem. Some reasons for short functions are:

Reusability

E.g. if you have a function, like a set method, you might assert it to be sure the parameters are valid. This checking has to be done everywhere the variable is set.

Maintainability

You might use some statement that you think in the future may change. E.g. you now show a symbol in a column, but later that might change to something else (or even a beep).

Uniformity

You are using e.g. the facade pattern and the only thing a function does is exactly passing the function to another without changing arguments.

Answer 14

When you give a code section a name, it is essentially for giving it a name. This can be for any of several reasons, but the crucial point is if you can give it a meaningful name which adds to your program. Names like "addOneToCounter" would not add anything, but conditionMet() would.

If you need a rule for finding out if the snippet is too short or too long, then consider how long time it takes you to find a meaningful name for the snippet. If you cannot within a reasonable amount of time, then the snippet is not a suitable size.

Answer 15

No, but it can be too terse.

Remember: Code is written once, but read many times.

Don't write code for the compiler. Write it for future developers who will have to maintain your code.

Answer 16

There's no code like no code!

Keep it simple and don't over complicate things.

It's not being lazy, it's doing your job!

Answer 17

Yes, its ok to have a short code function. In case of methods as "getters", "setters", "accesors" is very common, as previous answers mentioned.

Sometimes, those short "accesors" functions are virtual, because when overriden in subclasses the functions will have more code.

If you want you function not so short, well, in many functions, wheter global or methods, I usually use a "result" variable (pascal style) instead of a direct return, its very helpful when using a debugger.

function int CalculateSomething() {
  int Result = -1;

   // more code, maybe, maybe not

  return Result;
}

Answer 18

Yes dear, Function could be smaller and smaller and whether it is good or bad it depends on the Language/Framework you're using.

In my opinion I mostly work on Front End Technologies, Small Functions are mostly used as helper functions you'll be bound to use them like a lot when working with small filters and using same logic across your application. If your application has too much common logic then there are gonna be like a ton of small functions.

But in an application where you don't have common logic you won't be bound to make small functions but you can break your code into segments where it becomes easy for you to manage and understand.

In general break your huge code into small function is a very good approach. In modern frameworks and languages you'll be bound to do so e.g

data => initScroll(data)

is an anonymous function in ES 2017 JavaScript and Typescript

getMarketSegments() {
 this.marketService.getAllSegments(this.project.id)
  .subscribe(data => this.segments = data, error => console.log(error.toString()));
}

in the above code you can see 3 Function declaration and 2 function calls this is a simple Service Call in Angular 4 with Typescript. You can think of it as your requirements

([] 0)
([x] 1)
([x y] 2)

The above are 3 Anonymous functions in clojure Language

(def hello (fn [] "Hello world"))

The above one is a functional declaration in clojure

So yes FUNCTIONS can be as smaller but whether it is good or bad if you have functions like:

incrementNumber(numb) { return ++numb; }

Well it is not a good practice to do so but what if you're using this function in an HTML tag like we do in Angular Framework if there was no support for Increment or Decrement in Angular HTML Templates then this would have been the solution for me.

Lets take another example

insertInArray(array, newKey) {
 if (!array.includes(newKey)) {
   array.push(newKey);
 }
}

The above example is a must when playing when arrays inside Angular HTML Templates. So sometimes you'll have to create small functions

Answer 19

I disagree with almost answer given at this time.

Recently I found a colleague which write all classes members like the following:

 void setProperty(int value){ mValue=value; }
 int getProperty() const { return (mValue); }

This could be a good code in sporadic cases, but surely not if you define many classes having many and many properties. I don't want to say, with this, that methods like the ones above shall not be written. But, if you ends up to write most of the code as "wrapper" of the real logic, there is something wrong.

Probably the programmer miss the overall logic, fear about future changes and about refactoring the code.

The definition of the interface is because a real need; indeed if you need to set and get a simple property (without much logic, which makes the member shorter) it shall be possible. But, if the real need could be analysed in a different way, why don't define a more intuitive interface?

To really answer to the question, of course it is no, and the reason is very obvious: the method/property/whatelse shall be defined for what it needs. Even an empty virtual function has a reason to exists.