Is interleaving local variable declarations with assertions and function calls a bad practice?

Question

In my experience, it is customary to place local variable declarations at the beginning of their scope. Several questions in this forum ask whether this needs to be so, and their answers tend to agree on that variables should be declared nearest to their use. This answer suggests a goal of scope minimization, but does not state an opinion on whether declarations should be grouped together or not.

I do think it's cleaner to have declarations (a) grouped together and (b) at the start of their scope. But I also think that interleaving assertions and function calls might help (a) make functions fail faster in an error scenario and (b) add useful restrictions.

Making functions fail faster

Consider test_function_1 defined below:

void test_function_1(const int *const param)
{
    assert(NULL != param);

    /* (Function declarations.) */

    /* (Main logic.) */
}

Assume that the assertion is reasonable: test_function_1 needs param not to be null. Here, having the assertion precede the declarations helps the function fail faster from an irrecoverable scenario. I have not seen this style before, so I wonder:

Is there a reason not to do this?

Adding useful restrictions

Consider test_function_2 defined below:

void test_function_2(void)
{
    const int a = get_a(); 

    function_with_side_effects(a);

    const int b = function_affected_by_side_effects();

    /* (Main logic. Assume that 'a' and 'b' are used sufficiently.) */
}

Here, having the call to function_with_side_effects precede the declaration of b allows us to define b as a constant. (It is not possible to declare b as a constant, then call function_with_side_effects and then define b with function_affected_by_side_effects.) If we do things the traditional way and declare b first, not as a constant, then we lose the constraint that is so useful to us.

Is there a reason not to do this?

Answer 1

Is there a reason not to do this?

Tradition.

Stupid as it sounds we were forced to do things in certain orders in older languages and we're dragging those old, no longer relevant, habits around. Knowing this you can buck tradition but understand it comes at a cost. Computer don't care but a human will tend to be a stick in the mud. Get your team on board before you try to force change with code.

As for your specific examples:

Making functions fail faster

assert(NULL != param);

Assertions are intended for debugging code. They tend to get turned off when code is deployed. If you want this check in operational code consider a guard clause. Which is a fancy way to say just write an if. One that will send us elsewhere if things aren't right. Now this is c. No exceptions to throw. So all you can do is write to stderr and abort the program if you don't have an error handling block to go to. The nice thing about having them first is nothing got allocated yet so nothing needs de-allocation. There is a long tradition of putting guard clauses first.

Adding useful restrictions

Remember that a style guide is only a guide. What really counts is how your team feels. Personally I do not feel compelled to move every declaration to the top of a function. But when this comes up I start to suspect that a functions body is too long and needs decomposing.

Rather than fly directly at this problem you can choose to side step it. I know this is an example so my side step wont work for all the cases where you find this problem, but at least in this case there is nice way to avoid this issue:

int a_to_b(const int *const a)
{
    function_with_side_effects(a);
    return function_affected_by_side_effects();
}

With that this becomes a non-issue:

void test_function_2(void)
{
    const int a = get_a(); 
    const int b = a_to_b(a);

    /* (Main logic. Assume that 'a' and 'b' are used sufficiently.) */
}

Done this way the temporal coupling is explicit (you can't call a_to_b() without an a) and related code is still grouped together.

But yes, you can't always do that. I do not mind things getting declared near where they're used. I also do not mind short functions. So long as they have good names.