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

[Thiago Macieira]

On segunda-feira, 16 de dezembro de 2013 10:08:45, Linus Torvalds wrote:
> I'm no a fan of the "declare variables where they are used" model,
> because it tends to just encourage people to add crap to functions,
> and I think it makes it hard to see what the variables in a function
> are. For the kernel, we have that "if you need more than 5-10 local
> variables in a function, you're likely doing something wrong" model.
> Hiding variable declarations tends to just hide problems like that.

That's a good point. I don't mind having the variables declared at the top 
provided they are *few* variables. If there are tens of variables and the 
function is big enough I don't remember what this variable is or does, we have 
a problem either way. I solved it by moving the variables closer to the use-
point, but splitting the function is another way.

> But hey, it's personal. I much prefer to have local specialized inline
> functions and putting helper variables in those, for example. I would
> do that "Save the XML document into a zip file" as a helper function,
> for example, and *avoid* having that temporary buffer for the filename
> ("char filename[PATH_MAX];") in the middle of a larger function. And I
> would feel that *that* would add true readability, and that moving the
> variables to be mixed in with the code is actually a bad move.

Hmm... that's actually a cleaner solution, since right now we transfer a file 
name, which must then be reopened and deleted. It would be better if we could 
transfer a file handle of some sort.

The problem is that we can't do it with libzip: we can't give it a FILE* or 
file descriptor to write to, as zip_fdopen can only open in read-only mode. So 
we need to give it a temporary file name, which is insecure, and delete it 
later.

Still, it would make the thing a little cleaner if the file name were passed by 
the caller function. I can make that change easily. I can also modify the code 
to make it closer to your request. But I can't promise to always do it like 
that... muscle memory will kick in every now and then.

> So I feel - pretty strongly - that the whole C++ "mix code and
> declarations" model is crap, and largely brought on by bad coding
> styles that think that long functions that do several different things
> with lots of variables are somehow acceptable.

There are two other reasons to doing that way:

1) classes with constructors have a non-zero cost of initialisation. For the 
value-type Qt classes like QByteArray, QList and QString, the cost is very 
small: one atomic increment (Qt 4) or two comparisons (Qt 5).

2) for non-Qt classes, they may throw, so you don't want to initialise them 
until you actually need them.

C++ also often subscribes to the Resource Allocation Is Initialisation 
principle (RAII). I was going to do that for some of the code in my refactor, 
but thought better of it and just used gotos.

> Of course, having seen how painful it can be to just add a small
> random helper function (because you need to declare it in another file
> etc), I can see _why_ C++ people seem to do it. I still think it's a
> sign of a disease, but I've grumbled enough about it. In the end, I
> don't tend to touch UI code in C++, so why should you care what I
> think?

Well, there's an ongoing discussion right now for C++17 to be able to add 
random small helpers in the same class context without declaring it in the 
class header. If you hadn't mentioned this before, I would probably have 
ignored the thread when it first came up.

So how do you feel about having (indirectly) contributed to the C++ standard? 
:-)

The syntax is not yet set on stone, though.

	https://github.com/fmatthew5876/stdcxx-privext
	https://groups.google.com/a/isocpp.org/d/topic/std-proposals/xukd1mgd21I/discussion
	https://groups.google.com/a/isocpp.org/d/topic/std-proposals/jEJsK86r4JQ/discussion

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center
      PGP/GPG: 0x6EF45358; fingerprint:
      E067 918B B660 DBD1 105C  966C 33F5 F005 6EF4 5358
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20131216/b412fdee/attachment.sig>