[Proj] Code style in Proj
kreve at sdfe.dk
Sun Apr 22 10:10:23 EST 2018
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
- Don't have assignments hidden inside expressions
- 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
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.
> 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.
> 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
> Proj mailing list
> Proj at lists.maptools.org
More information about the Proj