Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

val != std::numeric_limits<double>::infinity() or !isinf(val) or isfinite(val)

This is likely bike-shedding, but maybe there is something interesting I'm missing...

If a class initializes a member val to std::numeric_limits<double>::infinity() and later wants to check if val has been changed to something valid (+/- inf are not valid here), what are the tradeoffs of these 3 approaches and did I miss any other interesting ways to solve this. (removed const for readability in the examples.)

bool IsInit() { return MinX != std::numeric_limits<double>::infinity(); }  // a

bool IsInit() { return !std::isinf(MinX); }  // b

bool IsInit() { return std::isfinite(MinX); }  // c

For now the code is C++03, but how would the options change with C++11, C++14 and C++17. e.g. with C++17, this code could be just std::optional<double> val. Or would quiet NaN be a safer bet just incase +/-inf became valid in the future?

This came up when I was reading this patch to this code:

  • https://github.com/OSGeo/gdal/commit/30194c640f403008625ff0c8be7aca155ac7ebe1
  • https://trac.osgeo.org/gdal/browser/trunk/gdal/ogr/ogr_core.h?rev=37821#L76

For easy reference:

  • std::numeric_limits::infinity()
  • std::isinf
  • std::isfinite
  • std::numeric_limits::quiet_NaN
  • std::optional

Sort of related:

  • is it safe to use std::numeric_limits::max() as a special “flag”?
like image 500
Kurt Schwehr Avatar asked Mar 21 '17 14:03

Kurt Schwehr


1 Answers

Using these special values as a flag for initialization is only valid so long as the variable cannot otherwise get to the special value. Infinity isn't that hard to come by in terms of mathematical operations as overflow will also get it. So that could be problematic. Using quiet NaN if it's available would be easier to use if all that the invariant is, is that this floating point value can't be set by the user to this value.

As far as the three approaches are concerned, only approach "a" exactly matches the initial value. So it's the most accurate approach if the invariant is to be that only that initial value represents being the initialized value. Of course none of these approaches do anything with respect to protecting the invariant to begin with, which is more the issue in my mind: whether or not the invariant is or can be effectively enforced.

While this isn't a code review site per-say, in terms of the linked code edit to the OGREnvelope class (which appears to just be an AABB) on github, I think a few things should be noted:

  • The IsInit - based on its code - appears meant to be returning whether the AABB has actually been set by the user. Not whether whether it's in its initial state/value as arguably IsInit suggests (and misled me to believe). Personally I prefer a name like IsValid for this test.
  • As to what the invariant actually is, the edit itself seems to recognize that a valid AABB should never have a MinX value that's greater than MaxX nor a MinY that's greater than MaxY. In other words, they should never be reversed in a valid AABB. MinX should always be less than or equal to MaxX (and same for min and max Y variables).
  • Using reversed infinity values as is done, makes it: (a) easier to potentially grow the AABB to envelop other AABBs (and the edit demonstrates what I mean by that in reducing the lines of code from 14 to 4); and (b) makes it capable of handling a wider range of valid AABBs than using the max() and lowest() values would.
  • The fact that the MinX, MaxX, MinY, MaxY member variables are publicly accessible means that IsInit and the invariant itself is only advisory in the sense that there's no encapsulation to protect the invariant.

In the broader context of these things being noted, NaN isn't appropriate. The invariant was never really that MinX != std::numeric_limits<double>::infinity(). And IsInit (as implemented in that code) is at best incomplete. In the context of it being renamed to something like IsValid, then a more logically consistent implementation is:

bool IsValid() const
{
    return !std::isnan(MinX) && !std::isnan(MinY) && MinX <= MaxX && MinY <= MaxY;
}

With this implementation, a valid AABB is an OGREnvelope whose values are all valid numbers (none of which are NaN) and whose min values must be less than or equal to the max values for X and Y respectively.

like image 143
Louis Langholtz Avatar answered Sep 18 '22 14:09

Louis Langholtz