Re: [dev] [st PATCH 1/3] xloadcols: remove cp variable

From: Roberto E. Vargas Caballero <k0ga_AT_shike2.com>
Date: Wed, 22 Apr 2015 09:05:59 +0200

mihail.zenkov:
> IMHO in this particular case index version much better for reading and
> understanding.

So, you think that is more understable:

        XftColorFree(xw.dpy, xw.vis, xw.cmap, &dc.col[i]);

than

        XftColorFree(xw.dpy, xw.vis, xw.cmap, cp);

Sorry, but I don't agree. The for part in both cases is idiomatic,
although in the case of cp could be written better like:

        for (cp = dc.col; cp < &dc.col[LEN(dc.col)]; ++cp)

If a C programmer has problems to understanding this idiomatic
pointer loop then he has a problem.

noname:
> It reuses the 'i' variable and makes loop style consistent across the
> function. It was more obvious before applying next two patches.

Auto varibles does not cost money, so don't worry about using
one or two more. In the other loops we need i because the index
position is used in some of the calculations, like for example:

        for(i = 16; i < 6*6*6+16; i++) {
                color.red = sixd_to_16bit( ((i-16)/36)%6 );
                color.green = sixd_to_16bit( ((i-16)/6) %6 );
                color.blue = sixd_to_16bit( ((i-16)/1) %6 );

but in the loop we are talking about, the only think we want is to
run over an array and call a function with the address of every
element of the array, and for this kind of loops I think the pointer
version is better.

This is the problem of the style patches, that depend a lot of
the style of every programmer, and it is something very, very
personal.

Regards,
Received on Wed Apr 22 2015 - 09:05:59 CEST