From 957b98a32afb6dea0c3c85a3e8ba144390bc3411 Mon Sep 17 00:00:00 2001
From: Pfiffikus <2845742+Pfiffikus@users.noreply.github.com>
Date: Thu, 26 Oct 2017 09:19:57 +0200
Subject: [PATCH] well-defined newEpsilon, target-epsilon, invisible default
 scale

What about to check for a well-defined newEpsilon (cf. http://realtimecollisiondetection.net/blog/?p=89)?
Apart from computational sciences view (cf. https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/ or https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html) I would prefer two slight modifications referring to https://en.wikipedia.org/wiki/Approximation_error#Formal_Definition:
The given epsilon should refer to the target value, otherwise the result would be unexpected, e.g. 101.02 == Approx(100).epsilon(0.01) gets true. The default scale should be invisible, thus, e.g. 101.01 == Approx(100).epsilon(0.01) gets false. Finally (both modifications accepted) even 101.000001 == Approx(100).epsilon(0.01) should get false, e.g.

To prevent a misuse of epsilon its setting should be checked.
---
 include/internal/catch_approx.hpp | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/internal/catch_approx.hpp b/include/internal/catch_approx.hpp
index 663dabf0..1d730a0c 100644
--- a/include/internal/catch_approx.hpp
+++ b/include/internal/catch_approx.hpp
@@ -25,7 +25,7 @@ namespace Detail {
         explicit Approx ( double value )
         :   m_epsilon( std::numeric_limits<float>::epsilon()*100 ),
             m_margin( 0.0 ),
-            m_scale( 1.0 ),
+            m_scale( 0.0 ),
             m_value( value )
         {}
 
@@ -53,7 +53,7 @@ namespace Detail {
         friend bool operator == ( const T& lhs, Approx const& rhs ) {
             // Thanks to Richard Harris for his help refining this formula
             auto lhs_v = double(lhs);
-            bool relativeOK = std::fabs(lhs_v - rhs.m_value) < rhs.m_epsilon * (rhs.m_scale + (std::max)(std::fabs(lhs_v), std::fabs(rhs.m_value)));
+            bool relativeOK = std::fabs( lhs - rhs.m_value ) < rhs.m_epsilon * (rhs.m_scale + std::fabs(rhs.m_value) );
             if (relativeOK) {
                 return true;
             }
@@ -97,6 +97,7 @@ namespace Detail {
 
         template <typename T, typename = typename std::enable_if<std::is_constructible<double, T>::value>::type>
         Approx& epsilon( T newEpsilon ) {
+            assert(newEpsilon >= 0.0f && newEpsilon < = 1.0f); // i.e. check for well-defined newEpsilon
             m_epsilon = double(newEpsilon);
             return *this;
         }
@@ -126,7 +127,7 @@ namespace Detail {
 
         friend bool operator == ( double lhs, Approx const& rhs ) {
             // Thanks to Richard Harris for his help refining this formula
-            bool relativeOK = std::fabs( lhs - rhs.m_value ) < rhs.m_epsilon * (rhs.m_scale + (std::max)( std::fabs(lhs), std::fabs(rhs.m_value) ) );
+            bool relativeOK = std::fabs( lhs - rhs.m_value ) < rhs.m_epsilon * (rhs.m_scale + std::fabs(rhs.m_value) );
             if (relativeOK) {
                 return true;
             }
@@ -162,6 +163,7 @@ namespace Detail {
         }
 
         Approx& epsilon( double newEpsilon ) {
+            assert(newEpsilon >= 0.0f && newEpsilon < = 1.0f); // i.e. check for well-defined newEpsilon
             m_epsilon = newEpsilon;
             return *this;
         }