[Proj] Code style in Proj

[Kristian Evers]

Hi Kurt,

You are brave to start a code style discussion :-) A few comments on your “rules”:

- Combine definition and declaration

It works in this example but I don’t think it can be enforced in more complicated code.
Variable declarations have to be made at the start of a block and that is not always possible.

- Add const

I don’t have a strong opinion on this one, although in this example I mostly think the “const double”
is cluttering up the code. What is gained by adding const here?

- For double literals, have at least one digit before and after decimal point.  e.g. .1 -> 0.1 and 3. -> 3.0

+1.

- Don't have assignments hidden inside expressions

Big +1!

- Convert #defines to typed const values

Would this be better as “static const …”?

- IWYU - Include what you use... here math.h

Another +1 here.


Finally, I get an error when compiling your code:

/Users/kevers/dev/proj4/src/PJ_august.c:25:16: error: initializer for aggregate is not a compile-time
      constant [-Werror,-Wc99-extensions]
        M * x1 * (3.0 + x12 - 3.0 * y12),
        ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

So for that you would have to assign xy.x and xy.y individually. Or possible make use of the proj_coord()
PJ_COORD initialiser function.

It would be interesting to see a more complicated function get the same treatment. 

/Kristian


> On 22 Apr 2018, at 15:08, Kurt Schwehr <schwehr at gmail.com> wrote:
> 
> Totally missed that lp.lam assignment!  Next iteration... also tried to add a const to the lp arg.
> 
> https://godbolt.org/g/b4FUf7
> 
> And as for the static analize, try to imagine that this is being done on a much more complicated function, because, yes, if a static analyzer didn't get this function, it's pretty much useless.
> 
> On Sun, Apr 22, 2018 at 5:26 AM, Even Rouault <even.rouault at spatialys.com> wrote:
> On samedi 21 avril 2018 15:46:22 CEST Kurt Schwehr wrote:
> > Hi all,
> >
> > I've been thinking about what is possible with the Proj code base with an
> > assumption that the code must be C89/C90 compatible. I played around for a
> > few in godbolt with PJ_august.c (because it's small) and ended up with
> > this. I tried to be aggressive as I could. I think my modified version is
> > likely to be more static analyzer friendly.
>  
> I hope they are intelligent enough to make sense of the original code ;-)
>  
> > - Don't have assignments hidden inside expressions
>  
> Definitely +1 on this !
>  
> To avoid the modification of lp.lam in the cos(),
>  
> > const double c = 1.0 + c1 * cos(lp.lam *= .5);
> > const double x1 = sin(lp.lam) * c1 / c;
>  
> this part could also be re-written, as
>  
> const double half_lam = 0.5 * lp.lam;
> const double c = 1.0 + c1 * cos(half_lam);
> const double x1 = sin(half_lam) * c1 / c;
>  
>  
>  
> --
> Spatialys - Geospatial professional services
> http://www.spatialys.com
> 
> 
> 
> -- 
> --
> http://schwehr.org
> _______________________________________________
> Proj mailing list
> Proj at lists.maptools.org
> http://lists.maptools.org/mailman/listinfo/proj