return an int or pass an int pointer -- whats better?

Question

which of these two is better?

void SetBit(int *flag, int bit)
{
    *flag |= 1 << bit;
}

Or

int SetBit(int flag, int bit)
{
    flag |= 1 << bit;
    return flag;
}

Answer 1

I like the second one because it doesn't have any side effects. If you want to modify flag, you can simply assign the result to itself:

flag = SetBit(flag, 4);

Answer 2

Neither.

int SetBit(int flag, int bit)
{
    return flag | 1 << bit;
}

Answer 3

I like the second one better...

However, I'd recommend changing the name of your function to be more descriptive (assuming this is the actual name). "SetBit" doesn't do much to describe what the function does or returns :)

Answer 4

It depends.

first one is imperative style, second one is functional style.

If you want to do

SetBit(SetBit(... SetBit(flag, b1), b2),...), bn)

do the second one. If you want

SetBit(&flag, b1)
SetBit(&flag, b2)
...
SetBit(&flag, bn)

do the first one. In C, I would prefer the latter (ie. the imperative one). In other languages/contexts, the former may be a good idea.

Answer 5

It depends.

In that case, either way would really work. But I can think of two special cases where I would favor using a pointer.

1. If the type you're passing in is large and a value copy would be expensive, use a pointer for performance reasons.

2. If you need to return something else, maybe a status code or success/failure indication, then you need to use the pointer so that you can leave room to return the value you need to return.

I personally think that outside of those situations, the second one (pass/return by value) is clearer and slightly more readable.

Answer 6

The second is better because it won't crash.

The first one will could crash if you pass in an NULL invalid pointer so you'd need to have some code to check and handle that.

Answer 7

pass an int to save time dereferencing the value of address

Answer 8

I would use a macro:

#define BIT_SET(a, bit) ((a) | (1 << (bit)))

Answer 9

To be honest, I think this just encourages people to use "magic numbers" as flags:

SetBit(&flags, 12); // 12 is the flag for Super mode

What you actually want is named constants:

#define SUPERMODE_FLAG 12
...
SetBit(&flags, SUPERMODE_FLAG);

But if you're going to use named constants, you might as well name masks rather than bit numbers, in which case the operation is so simple there's no need for a helper function:

#define SUPERMODE_MASK (1 << 12)
....
flags |= SUPERMODE_MASK;

In the unusual case that you're manipulating individual bits by number, without knowing what they mean, then I prefer the second for the same reason as Kristo - I find side-effect-free functions slightly easier to reason about than mutators.

Answer 10

First is OK if functions is going to be inlined. Without inlining that's bit too much overhead to pass around pointers to ints. (On 64bit LP64 archs, int is 4 bytes, pointer is 8.)

Second ... function name SetBit() is going to cause some mild confusion. Name implies that function changes something while in fact it doesn't. As long as you are OK with the name, then it is performance-wise a better option.

E.g. Linux kernel uses for many similar things the pointer variant, since often memory location of the datum is important or required for portability. But they either make all such functions a preprocessor macro or mark with gcc's always_inline attribute. For user-land, plain application programming, I'd say the second should be preferred. Only pick a better name.

Answer 11

If the only pattern for using the function will be "variable = doSomething(variable);" and performance is not an issue, I would consider "doSomething(&variable);" to be more legible. The only time I would favor the former would be if the destination was sometimes something other than the source, or if performance were crucial and one's compiler could not efficiently handle the latter case (common on embedded systems).

It should be noted that the latter format allows something the former does not. In VB-style:

Sub OrBits(ByRef N as Integer, ByVal Bits as Integer)
  Dim OldValue as Integer
  Do
    OldValue = N
  Loop While Threading.Interlocked.CompareExchange(N, OldValue or Bits,  OldValue) <> OldValue
End Sub

The effect of this code will always be to OR the specified bits into N, even if something else changes N while the function is running. It is not possible to achieve such behavior with the read-and-return strategy.