views:

372

answers:

8

What do you think about one line functions? Is it bad? One advantage I can think of is that it makes the code more comprehensive (if you choose a good name for it). For example:

void addint(Set *S, int n)
{
     (*S)[n/CHAR_SIZE] |= (unsigned char) pow(2, (CHAR_SIZE - 1) - (n % CHAR_SIZE));
}

One disadvantage I can think of is that it slows the code (pushing parameters to stack, jumping to a function, popping the parameters, doing the operation, jumping back to the code - and only for one line?)

is it better to put such lines in functions or to just put them in the code? Even if we use them only once?

BTW, I haven't found any question about that, so forgive me if such question had been asked before.

+1  A: 

If you use the code within that function 3 times or more, then I would recommend to put that in a function. Only for maintainability.

Ólafur Waage
+5  A: 

If used more than once, definitely make it a function, and let the compiler do the inlining (possibly adding "inline" to the function definition). (<Usual advice about premature optimization goes here>)

Brian
+1  A: 

What language? If you mean C, I'd also use the inline qualifier. In C++, I have the option of inline, boost.lamda or and moving forward C++0x native support for lamdas.

dirkgently
+5  A: 

Since your example appears to be using a C(++) syntax you may want to read up on inline functions which eliminate the overhead of calling a simple function. This keyword is only recommendation to the compiler though and it may not inline all functions that you mark, and may choose to inline unmarked functions.

In .NET the JIT will inline methods that it feels is appropiate, but you have no control over why or when it does this, though (as I understand it) debug builds will never inline since that would stop the source code matching the compiled application.

Martin Harris
+6  A: 

I am not a fan of having all sort of logic and functionality banged into one line. The example you have shown is a mess and could be broken down into several lines, using meaningful variable names and performing one operation after another.

I strongly recommend, in every question of this kind, to have a look (buy it, borrow it, (don't) download it (for free)) at this book: Robert C. Martin - Clean Code. It is a book every developer should have a look at.

It will not make you a good coder right away and it will not stop you from writing ugly code in the future, it will however make you realise it when you are writing ugly code. It will force you to look at your code with a more critical eye and to make your code readable like a newspaper story.

Peter Perháč
+2  A: 

There is nothing wrong with one line functions. As mentioned it is possible for the compiler to inline the functions which will remove any performance penalty.

Functions should also be preferred over macros as they are easier to debug, modify, read and less likely to have unintended side effects.

If it is used only once then the answer is less obvious. Moving it to a function can make the calling function simpler & clearer by moving some of the complexity into the new function.

Steven
A: 

Sometimes it's not a bad idea to use the preprocessor:

#define addint(S, n) (*S)[n/CHAR_SIZE] |= (unsigned char) pow(2, (CHAR_SIZE - 1) - (n % CHAR_SIZE));

Granted, you don't get any kind of type checking, but in some cases this can be useful. Macros have their disadvantages and their advantages, and in a few cases their disadvantages can become advantages. I'm a fan of macros in appropriate places, but it's up to you to decide when is appropriate. In this case, I'm going to go out on a limb and say that, whatever you end up doing, that one line of code is quite a bit.

#define addint(S, n) do { \
    unsigned char c = pow(2, (CHAR_SIZE -1) - (n % CHAR_SIZE)); \
    (*S)[n/CHAR_SIZE] |= c \
  } while(0)
Chris Lutz
Quoth the preprocessor: "Abuse me, Abuse me, Make me feel wanted!"
Tim Post
I just felt like it should be mentioned for completeness. It may not be the best solution in this case, because there's no type checking, but I do mention that. I'm not saying "(ab)use the preprocessor", I'm saying "don't forget about the preprocessor". It's not THAT bad of a tool.
Chris Lutz
I agree that the preprocessor is a very useful tool. However, when something gets that complex, its often better to inline it, look at the asm dump and see if your compiler agreed. 8/10 times, the compiler is going to be correct. Now, if your compiler is just being an A** hole and you know it, there remains the preprocessor.
Tim Post
+15  A: 

Don't be scared of 1-line functions!

A lot of programmers seem to have a mental block about 1-line functions, you shouldn't.

If it makes the code clearer and cleaner, extract the line into a function.

Performance probably won't be affected.

Any decent compiler made in the last decade (and perhaps further) will automatically inline a simple 1-line function. Also, 1-line of C can easily correspond to many lines of machine code. You shouldn't assume that even in the theoretical case where you incur the full overhead of a function call that this overhead is significant compared to your "one little line". Let alone significant to the overall performance of your application.

Abstraction Leads to Better Design. (Even for single lines of code)

Functions are the primary building blocks of abstract, componentized code, they should not be neglected. If encapsulating a single line of code behind a function call makes the code more readable, do it. Even in the case where the function is called once. If you find it important to comment one particular line of code, that's a good code smell that it might be helpful to move the code into a well-named function.

Sure, that code may be 1-line today, but how many different ways of performing the same function are there? Encapsulating code inside a function can make it easier to see all the design options available to you. Maybe your 1-line of code expands into a call to a webservice, maybe it becomes a database query, maybe it becomes configurable (using the strategy pattern, for example), maybe you want to switch to caching the value computed by your 1-line. All of these options are easier to implement and more readily thought of when you've extracted your 1-line of code into its own function.

Maybe Your 1-Line Should Be More Lines.

If you have a big block of code it can be tempting to cram a lot of functionality onto a single line, just to save on screen real estate. When you migrate this code to a function, you reduce these pressures, which might make you more inclined to expand your complex 1-liner into more straightforward code taking up several lines (which would likely improve its readability and maintainability).

Wedge