Thoughts on Special Function atanh

From: Daryle Walker (darylew_at_[hidden])
Date: 2001-05-19 07:28:07

This function is from the Special Functions library currently under review.

The Issues:

1.  Clutter.  That can be cleaned up with reducing all the static casts to a
single constant.

2.  Nested If's.  The if-else structure isn't the most efficient.  The inner
structure has an empty else case because the outer structure already takes
care of it.  Those structures can be combined into one.

3.  Runtime handling of compile-time stuff.  The original code branches
between using a function and throwing an exception based on a Boolean
constant that should be available at compile-time.  This is inefficient and
may cause a problem in that a non-optimizing compiler may always make a
reference to the function choice, even though the exception choice will be
made (real bad since the reference would be unresolved).

4.  Test coverage.  The test file doesn't cover the outside range checks,
neither for NaN nor exception returns.  Exception returns for the +/-1 case
aren't checked either.  (Maybe instantiate something for "int"?)

Here's an alternative version with the first three fixes:

//=========================================================================
#include <cmath>
#include <limits>
#include <string>
#include <stdexcept>

namespace boost
{
// These are implementation details
namespace detail
{
template < typename T, bool InfinitySupported >
struct atanh_helper1_t
{
    static  T  get_pos_infinity()
        { return +std::numeric_limits<T>::infinity(); }
    static  T  get_neg_infinity()
        { return -std::numeric_limits<T>::infinity(); }

};  // boost::detail::atanh_helper1_t

template < typename T >
struct atanh_helper1_t< T, false >
{
    static  T  get_pos_infinity()
    {
        char const  error_str[] = "atanh(+1) == +Infinity";
        throw std::out_of_range( static_cast<std::string>(error_str) );
    }

    static  T  get_neg_infinity()
    {
        char const  error_str[] = "atanh(-1) == -Infinity";
        throw std::out_of_range( static_cast<std::string>(error_str) );
    }
};  // boost::detail::atanh_helper1_t

template < typename T, bool QuietNanSupported >
struct atanh_helper2_t
{
    static  T  get_pos_NaN()
        { return +std::numeric_limits<T>::quiet_NaN(); }
    static  T  get_neg_NaN()
        { return -std::numeric_limits<T>::quiet_NaN(); }

};  // boost::detail::atanh_helper2_t

template < typename T >
struct atanh_helper2_t< T, false >
{
    static  T  get_pos_NaN()
    {
        char const  error_str[] = "atanh(x) forbidden for x > +1";
        throw std::domain_error( static_cast<std::string>(error_str) );
    }

    static  T  get_neg_NaN()
    {
        char const  error_str[] = "atanh(x) forbidden for x < -1";
        throw std::domain_error( static_cast<std::string>(error_str) );
    }

};  // boost::detail::atanh_helper2_t

}  // boost::detail

// This is the inverse of the hyperbolic tangent function.
template<typename T>
inline T    atanh(const T x)
{
    typedef detail::atanh_helper1_t<T, std::numeric_limits<T>::has_infinity>
      helper1_type;
    typedef detail::atanh_helper2_t<T,
      std::numeric_limits<T>::has_quiet_NaN>  helper2_type;

    T const  one = static_cast<T>( 1 );

    if ( x < -one )
    {
        return helper2_type::get_neg_NaN();
    }
    else if ( x == -one )
    {
        return helper1_type::get_neg_infinity();
    }
    else if ( x < +one )
    {
        using std::log;
        return log( (one + x) / (one - x) ) / ( one + one );
    }
    else if ( x == +one )
    {
        return helper1_type::get_pos_infinity();
    }
    else  // x > +one
    {
        return helper2_type::get_pos_NaN();
    }
}
}  // boost
//=========================================================================

You should note I changed the exception type of the |x| > 1 cases.  The
out_of_range error should be used when an answer exists, but is unreachable
(|x| == 1).  The domain_error is more proper when the answer is nonsensical.

The amount of C-style comments should be minimized, to allow the trick of
commenting out a bunch of code at once.  (It can't work when C-style
comments abound, since C-style comments don't nest.)  The reverse of another
function is called "inverse," not "reciprocal."

-- 
Daryle Walker
Mac, Internet, and Video Game Junkie
darylew AT mac DOT com