C functions overusing parameters?

Question

I have legacy C code base at work and I find a lot of function implementations in the style below.

char *DoStuff(char *inPtr, char *outPtr, char *error, long *amount)
{
    *error = 0;
    *amount = 0;

    // Read bytes from inPtr and decode them as a long storing in amount
    // before returning as a formatted string in outPtr.

    return (outPtr);
}

Using DoStuff:

myOutPtr = DoStuff(myInPtr, myOutPtr, myError, &myAmount);

I find that pretty obtuse and when I need to implement a similar function I end up doing:

long NewDoStuff(char *inPtr, char *error)
{
    long amount = 0;
    *error = 0;

    // Read bytes from inPtr and decode them as a long storing in amount.

    return amount;
}

Using NewDoStuff:

myAmount = NewDoStuff(myInPtr, myError);
myOutPtr += sprintf (myOutPtr, "%d", myAmount);

I can't help but wondering if there is something I'm missing with the top example, is there a good reason to use that type of approach?

Answer 1

It is the C standard library style. The return value is there to aid chaining of function calls.

Also, DoStuff is cleaner IMO. And you really should be using snprintf. And a change in the internals of buffer management do not affect your code. However, this is no longer true with NewDoStuff.

Answer 2

The code you presented is a little unclear (for example, why are you adding myOutPtr with the results of the sprintf.

However, in general what it seems that you're essentially describing is the breakdown of one function that does two things into a function that does one thing and a code that does something else (the concatenation).

Separating responsibilities into two functions is a good idea. However, you would want to have a separate function for this concatenation and formatting, it's really not clear.

In addition, every time you break a function call into multiple calls, you are creating code replication. Code replication is never a good idea, so you would need a function to do that, and you will end up (this being C) with something that looks like your original DoStuff.

So I am not sure that there is much you can do about this. One of the limitations of non-OOP languages is that you have to send huge amounts of parameters (unless you used structs). You might not be able to avoid the giant interface.

Answer 3

If you wind up having to do the sprintf call after every call to NewDoStuff, then you are repeating yourself (and therefore violating the DRY principle). When you realize that you need to format it differently you will need to change it in every location instead of just the one.

Answer 4

One advantage is that if you have many, many calls to these functions in your code, it will quickly become tedious to have to repeat the sprintf calls over and over again.

Also, returning the out pointer makes it possible for you to do things like:

DoOtherStuff(DoStuff(myInPtr, myOutPtr, myError, &myAmount), &myOther);

With your new approach, the equivalent code is quite a lot more verbose:

myAmount = DoNewStuff(myInPtr, myError);
myOutPtr += sprintf("%d", myAmount);
myOther  = DoOtherStuff(myInPtr, myError);
myOutPtr += sprintf("%d", myOther);

Answer 5

As a rule of thumb, if the interface to one of my functions exceeds 110 columns, I look strongly at using a structure (and if I'm taking the best approach). What I don't (ever) want to do is take a function that does 5 things and break it into 5 functions, unless some functionality within the function is not only useful, but needed on its own.

I would favor the first function, but I'm also quite accustomed to the standard C style.