HiveBrain v1.2.0
Get Started
← Back to all entries
principlecppMinor

Compare two doubles to N decimal places

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
placesdecimaldoublestwocompare

Problem

I have a function that compares two doubles to N decimal places.

#include 
#include 

using namespace std;

enum class RelType : int { Less, Equal, Greater };

RelType _compare(double v1, double v2, int multiplier)
{
    if (std::round(v1 * multiplier) == std::round(v2 * multiplier))
        return RelType::Equal;
    if (v1  RelType compare(double, double) { static_assert(false, "Not implemented."); }
template<> RelType compare(double v1, double v2) { return _compare(v1, v2, 100000); }

int main(int argc, char* argv[])
{
    double v1 = 1.0;                        // 1.0000000000000000
    double v2 = (1.0 / 13.67) * 13.67;      // 0.99999999999999989

    cout (v1, v2)==RelType::Equal)  v2) (v1, v2)==RelType::Greater) << endl;   // false (correct behaviour)

    return 0;
}


My main two questions are about the efficiency and correctness of the _compare helper function. Are there any pitfalls to consider that might not be obvious at first glance?

Solution

The main problem I have with your code is with these templates:

template RelType compare(double, double) { static_assert(false, "Not implemented."); }
template<> RelType compare(double v1, double v2) { return _compare(v1, v2, 100000); }


They are misleading. The 5 in the second function should be supposed to denote the number of decimal places to be compared, however, it is not implemented to do so, being only used as name decoration. E.g.:

// Comparing 5 decimal places:
cout (v1, v2) == RelType::Equal) (v1, v2) == RelType::Equal) << endl;


I see no point in having a parameter if it always has to have the same value, so one improvement to the interface would be just getting rid of the templates and having a single function that always compares with 5 decimal places and no other option.

Another possibility, if you need to have the ability to configure the number of decimal palaces at compile time, would be actually using the N parameter to compute the multiplier, instead of hardcoding a value. You can do so with a compile-time power function:

template RelType compare(double v1, double v1)
{
    constexpr auto multiplier = compile_time_pow(10, N);
    // ... the rest ...
}


Note that in the above link C++11 is utilized, though it is probably possible to achieve the same without constexpr.

Code Snippets

template<int N> RelType compare(double, double) { static_assert(false, "Not implemented."); }
template<> RelType compare<5>(double v1, double v2) { return _compare(v1, v2, 100000); }
// Comparing 5 decimal places:
cout << (compare<5>(v1, v2) == RelType::Equal) << endl;

// Also comparing 5 decimal places:
// (In actuality, a shady compile error due 
// to the static_assert. But we can do better...)
cout << (compare<8>(v1, v2) == RelType::Equal) << endl;
template<int N> RelType compare(double v1, double v1)
{
    constexpr auto multiplier = compile_time_pow(10, N);
    // ... the rest ...
}

Context

StackExchange Code Review Q#82958, answer score: 7

Revisions (0)

No revisions yet.