Storing currency-precision values

Question

I'm trying to develop a program that stores currency values. In my particular application I only care about two decimal of precision (cents) but have read it's a good idea (for accuracy when dealing with interest rates) to store 4 digits.

I didn't notice a "currency" class in either boost or C++14 (perhaps I overlooked, if so please point out). The main motivation for me using a class instead of just "double" is I was concerned about inaccuracies that, in my experience, seem to pop up in the last 3-4 digits stored by doubles when multiplying / dividing.

I don't have any locale-specific features included because it seems, at least as far as string formatting, boost does have a way to deal with that problem.

Please provide feedback on this class. Any alterations that should be made / best practices, etc.

currency.hpp

#pragma once
#include <type_traits>
#include <stdint.h>

class currency {
private:
    static constexpr long double FACTOR = 10000;
    typedef int64_t p_int;
    p_int value;

public:
    long double round() {
        p_int num = value / 100;
        int8_t digs = value % 100;
        if (digs >= 50) { ++num; }
        return (long double)(num) / 100;
    }

    // Destructor and Constructors
    ~currency() {}
    currency() { value = 0; }
    currency(const currency& obj) { value = obj.value; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    currency(const N& n) { value = n * FACTOR; }

    // Assignment operator
    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    currency& operator=(const N& n) { value = n * FACTOR; return *this; }
    currency& operator=(const currency& n) { value = n.value; return *this; }

    // Comparison operators
    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    bool operator==(const N& n) { return value == (p_int)(n * FACTOR); }
    bool operator==(const currency& n) { return value == n.value; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    bool operator<(const N& n) { return value < (p_int)(n * FACTOR); }
    bool operator<(const currency& n) { return value < n.value; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    bool operator<=(const N& n) { return value <= (p_int)(n * FACTOR); }
    bool operator<=(const currency& n) { return value <= n.value; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    bool operator>(const N& n) { return value > (p_int)(n * FACTOR); }
    bool operator>(const currency& n) { return value > n.value; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    bool operator>=(const N& n) { return value >= (int)(n * FACTOR); }
    bool operator>=(const currency& n) { return value >= n.value; }

    // Compound Arithmetic operators
    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    currency& operator+=(const N& n) { value += n * FACTOR; return *this; }
    currency& operator+=(const currency& n) { value += n.value; return *this; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    currency& operator-=(const N& n) { value += n * -FACTOR; return *this; }
    currency& operator-=(const currency& n) { value -= n.value; return *this; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    currency& operator/=(const N& n) { value /= n; return *this; }
    currency& operator/=(const currency& n) { value /= (n.value / FACTOR); return *this; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    currency& operator*=(const N& n) { value *= n; return *this; }
    currency& operator*=(const currency& n) { value *= (n.value / FACTOR); return *this; }

    // Arithmetic Operators
    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    currency operator+(const N& n) { currency t(*this); return t += n; }
    currency operator+(const currency& n) { currency t(*this); return t += n; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    currency operator-(const N& n) { currency t(*this); return t -= n; }
    currency operator-(const currency& n) { currency t(*this); return t -= n; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    currency operator/(const N& n) { currency t(*this); return t /= n; }
    currency operator/(const currency& n) { currency t(*this); return t /= n; }

    template <typename N,
        typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
    currency operator*(const N& n) { currency t(*this); return t *= n; }
    currency operator*(const currency& n) { currency t(*this); return t *= n; }
};

currency.cpp

#include "currency.hpp"
constexpr long double currency::FACTOR;

Answer 1

I've only a few small remarks:

1. FACTOR should be an integer and not a floating point number, as this promotes all computations to floating point computations whose results you then convert back to integer values. Aside from the performance implications, this is NOT always a lossless operation (depending on the sums involved.
2. You don't have to explicitly define the destructor. The default one will do just fine.
3. There is no need to implement all operators as template functions. As you have already implemented a conversion constructor for all arithmetic types (actually I'd rather make that constructor explicit), a single operator version taking a currency object as an argument should suffice. So for operators +, +=, -, -=, <,>,==,>=,<= the non-templated version should suffice.

4. On the other hand I'd not implement multiplication with a currency parameter, because it doesn't really make sense to multiply money with money. Also dividing money by money should have e.g. a floating point number as a result.:

   // Compound Arithmetic operators    
   // ... += and -=
   
   template <typename N, typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
   currency& operator/=(const N n) { value /= n; return *this; }       
   
   template <typename N, typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
   currency& operator*=(const N n) { value *= n; return *this; }       
   
   // Arithmetic Operators
   template <typename N, typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
   currency operator/(const N n) { currency t(*this); return t /= n; }
   
   template <typename N, typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
   currency operator*(const N n) { currency t(*this); return t *= n; }
   
   float operator/(const currency n) { return this->value / n.value; }
   

   At least in my mindset currency is similar to physical units and the same way, as dividing seconds by seconds doesn't result in seconds, but a dimension less number, I think the same should hold true for currency.

5. No need to pass currency parameters by reference. As your classes' only member is a single integer, pass by value is at least as efficient here.

6. If you want to allow implicit conversions from floating point numbers, you should consider introducing a range check and throw an exception if the number lies outside of the allowed range of p_int e.g.:

   template <typename N, typename std::enable_if<std::is_arithmetic<N>::value>::type* = nullptr>
   currency(const N n) { 
       //DEBUG Check:
       assert(n < std::numeric_limits<p_int>::max() / FACTOR);
       assert(n > std::numeric_limits<p_int>::min() / FACTOR);
   
       //Production Check
       if (n > std::numeric_limits<p_int>::max() / FACTOR ||
           n < std::numeric_limits<p_int>::min() / FACTOR ) {
           throw(std::range_error("Value cannot be represented by currency class"));
       }
       value = n * FACTOR; 
   }
   

   However, be aware that this might introduce a noticeable runtime overhead. Considering that you will most likely not deal with amounts that exceed a trillion dollars anyway (which is still well below the maximum value a 64bit int can handle with your specified FACTOR). I think it is reasonable to omit that runtime check and state it as a precondition of your functions.

Btw: Your decision not to use a double was absolutely correct. A floating point variable just can't exactly represent e.g "0.1" Dollars. It can do it with a very high precision, but not exactly.

EDIT: To elaborate a bit more on point 5): Most likely it will not make any difference, because the compiler will inline the functions anyway. If not, pass by value might be more efficient, because a reference is essentially just a pointer. So pass by reference requires at least one store and one load of the used value (the address itself is passed in the register). Pass by value on the other hand sometimes allows the value to stay in the registers. It sometimes also eases compiler optimizations, because the compiler doesn't have to worry about aliasing. But again - most likely this will not make any difference. Passing built-in types by value its just something I do by default.

Answer 2

To add the suggestions made by @MikeMB:

1. Move the initialization of value outside the body of the default constructor and put it in the initialization list.

   currentcy() : value(0) {}
   

2. Remove the explicitly defined copy constructor. The compiler generated copy constructor will work just fine.. You'll need it since you have an overloaded templated constructor.

3. You can simplify the logic of rounding.

   long double round() {
       p_int num = (value + 50) / 100;
       return (long double)(num) / 100;
   }