Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid potentially very long loop when normalizing longitudes around long_wrap_center #518

Merged
merged 1 commit into from
May 23, 2017

Conversation

rouault
Copy link
Member

@rouault rouault commented May 23, 2017

PIN->long_wrap_center = pj_param(ctx, start, "rlon_wrap").f;
/* Don't accept excessive values otherwise we might perform badly */
/* when correcting longitudes around it */
if( !(fabs(PIN->long_wrap_center) < 10 * M_TWOPI) )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not equivalent to if ( fabs(PIN->-long_wrap_center) > 10 * M_TWOPI ) ? A little easier to read I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not completely... In case long_wrap_center is NaN. fabs(NaN) > bla is equivalent to NaN > bla , and comparisons involving NaN are always false

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bloody NaN's! Allright, carry on :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another indication that we need a perl-like "unless" construct in C (as in #define unless(x) if (x){;} else, or (less safe) #define unless(x) if (!(x))

There are quite a few expressions which are way easier to grasp mentally if stated in negated logic :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a formulaic comment "Backwards test to allow NaNs to satisfy the condition" is called for.

@rouault rouault merged commit adc0331 into OSGeo:master May 23, 2017
@rouault
Copy link
Member Author

rouault commented May 28, 2017

Perhaps a formulaic comment "Backwards test to allow NaNs to satisfy the condition" is called for.

Agreed. Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants