What are the advantages of squashing assignment and error checking in one line?

This question is inspired by this question, which features the following code snippet.

int s;
if((s = foo()) == ERROR)
    print_error();

I find this style hard to read and prone to error (as the original question demonstrates -- it was prompted by missing parentheses a开发者_运维技巧round the assignment). I would instead write the following, which is actually shorter in terms of characters.

int s = foo();
if(s == ERROR)
    print_error();

This is not the first time I've seen this idiom though, and I'm guessing there are reasons (perhaps historical) for it being so often used. What are those reasons?

I think it's for hysterical reasons, that early compilers were not so smart at optimizing. By putting it on one line as a single expression, it gives the compiler a hint that the same value fetched from foo() can be tested rather than specifically loading the value from s.

I prefer the clarity of your second example, with the assignment and test done later. A modern compiler will have no trouble optimizing this into registers, avoiding unnecessary loads from memory store.

When you are writing a loop, it is sometimes desirable to use the first form, as in this famous example from K&R:

int c;

while ((c = getchar()) != EOF) {
    /* stuff */
}

There is no elegant "second-form" way of writing this without a repetition:

int c = getchar();

while (c != EOF) {
    /* stuff */
    c = getchar();
}

Or:

int c;

for (c = getchar(); c != EOF; c = getchar()) {
    /* stuff */
}

Now that the assignment to c is repeated, the code is more error-prone, because one has to keep both the statements in sync.

So one has to be able to learn to read and write the first form easily. And given that, it seems logical to use the same form in if conditions as well.

I tend to use the first form mostly because I find it easy to read—as someone else said, it couples the function call and the return value test much more closely.

I make a conscious attempt at combining the two whenever possible. The "penalty" in size isn't enough to overcome the advantage in clarity, IMO.

The advantage in clarity comes from one fact: for a function like this, you should always think of calling the function and testing the return value as a single action that cannot be broken into two parts ("atomic", if you will). You should never call such a function without immediately testing its return value.

Separating the two (at all) leads to a much greater likelihood that you'll sometimes skip checking the return value completely. Other times, you'll accidentally insert some code between the call and the test of the return value that actually depends on that function having succeeded. If you always combine it all into a single statement, it (nearly) eliminates any possibility of falling into these traps.

I would always go for the second. It's easier to read, there's no danger of omitting the parentheses around the assignment and it is easier to step through with a debugger.

I often find the separation of the assignment out into a different line makes debugger watch or "locals" windows behave better vis-a-vis the presence and correct value of "s", at least in non-optimized builds.

It also allows the use of step-over separately on the assignment and test lines (again, in non-optimized builds), which can be helpful if you don't want to go mucking around in disassembly or mixed view.

YMMV per compiler and debugger and for optimized builds, of course.

I personally prefer for assignments and tests to be on different lines. It is less syntactically complicated, less error prone, and more easily understood. It also allows the compiler to give you more precise error/warning locations and often makes debugging easier.

It also allows me to more easily do things like:

int rc = function();

DEBUG_PRINT(rc);

if (rc == ERROR) {
    recover_from_error();
} else {
    keep_on_going(rc);
}

I prefer this style so much that in the case of loops I would rather:

while (1) {
    int rc = function();
    if (rc == ERROR) {
        break;
    }
    keep_on_going(rc);
}

than do the assignment in the while conditional. I really don't like for my tests to have side-effects.

I often prefer the first form. I couldn't say exactly why, but it has something to do with the semantic involved.

The second style feels to me more like 2 separate operations. Call the function and then do something with the result, 2 different things. In the first style it's one logical unit. Call the function, save the temprary result and eventually handle the error case.

I know it's pretty vague and far from being completely rational, so I will use one or the other depending on the importance of the saved variable or the test case.

I believe that clarity should always prime over optimizations or "simplifications" based only on the amount of characters typed. This belief has stopped me from making many silly mistakes.

Separating the assignement and the comparison makes both clearer and so less error-prone, even if the duplication of the comparison might introduce a mistake once in a while. Among other things, parentheses become quickly hard to distinguish and keeping everything on one line introduces more parentheses. Also, splitting it up limits statements to doing only one of either fetching a value or assigning one.

However, if you expect people who will read your code to be more comfortable using the one-line idiom, then it is wide-spread enough not to cause any problems for most programmers. C programmers will definately be aware of it, even those that might find it awkward.