[PATCH 4/4] Move the variable initialisations closer to use and add blanks

Linus Torvalds torvalds at linux-foundation.org
Mon Dec 16 10:45:21 UTC 2013


On Mon, Dec 16, 2013 at 10:25 AM, Dirk Hohndel <dirk at hohndel.org> wrote:
> On Mon, 2013-12-16 at 10:08 -0800, Linus Torvalds wrote:
>
> The questions becomes one of "how can we be consistent"? I'm a big
> believer in consistency to make the code look the way you expect it to
> look. That includes things like variable naming schemes, coding style,
> indentation, but also "where are things declared"...
>
> So you think loop variables are fine to be declared right in the loop,
> but other variables should be at the top?

Actually, even in the loop, it's very much at the top.

So I'm not saying that all variables should be declared at the top of
the *function*. I'm saying they should be declared at the top of the
*scope*, and be very clear within that scope.

And loops are scopes of their own. So this is fine:

   int function(int n, const struct xyz *ptr)
   {
       int i,j;

       .. code ..
       if (something) {
          void *ptr;

          .... code in this scope uses ptr..
      }

because the "void *ptr" isn't *mixed* with code, it's clearly separate
from code within that scope. Limiting the scope of a variable as much
as possible is good, because then you also see - very obviously -
where it's used and where it is not.

In contrast, putting the declaration in the middle of code makes the
variable hard to pick out, and while you limit where it is used (it
can only be used *after* the declaration), it's even better to limit
it to an even smaller scope (where there's both a begin and end limit,
not just a begin limit).

And loops are special, because they are their own scope. But you can't
declare the counting variable *inside* the loop, since it's part of
the loop control, so

      for (int i = 0; i < 10; i++) {
            ...
      }

is about as close to that ideal "top of scope" as you can get. It's
clear that "i" is only used for that loop.

(Historical note: originally, C++ got *this* wrong too, and the scope
of "i" extended to outside and after the loop. Which is moronic and
shows - yet again - how bad taste C++ people had. Happily, they fixed
it, and the scope of "i" is now limited to just the loop, although in
some compilers you have to use the "I'm modern" flag to enable proper
loop variable scoping).

So I don't argue that all variables have to be at the top of a
function, just that declaring them mixed *inside* the code is often
bad form and a sign of other issues.

Don't get me wrong, it can be convenient, and if there are good
reasons for it it's not necessarily a bad idea.

It's just that most of the reasons I ever see for it are very much
*not* good (ie they are very much the "we have so many local variables
and our functions are so big, that we want to move the declaration
where it's visible from the use"), and then I think the fix is not to
have the declaration mixed with the code, but to try to make the
function smaller and use helper inline functions whenever humanly
possible instead.

               Linus


More information about the subsurface mailing list