2
\$\begingroup\$

I was interesting in having a way to convert between dollars and cents without getting into floating point numbers and losing type safety. It's very easy to get confused when you're mixing numbers with concepts. Right now what I've built allows the following:

dollars d{1};
cents   c{100};
dollars d2{c};
dollars d3{2};
cents   c2{200};
std::cout << (d == c) 
          << " " 
          << d2.count()
          << " "
          << (d3 == c2);

So far I've only implemented operator==. To get this far, I also had to provide gcd since it's lacking from the standard library, which meant also providing a constexpr abs and a meta template to enforce integral arguments for gcd. I also had to specialize common_type (which is allowed and defined behavior).

Right now, some parts of the code are unwieldy, especially when calculating the gcd between two ratios. Please help me:

  • improve readability

  • simplify the template code

Full code dump follows:

#include <iostream>
#include <ratio>
#include <type_traits>

#include <cstdint>

namespace utils
{
    template <typename T>
    constexpr auto abs(T i) -> std::enable_if_t<std::is_integral<T>{}(), T>
    {
        return i < T(0) ? -i : i;
    }

    template <typename M, typename N = M>
    using common_int_t = std::enable_if_t<
        std::is_integral<M>{}() &&
        std::is_integral<N>{}(),
        std::common_type_t<M, N>
    >;

    template <typename M, typename N>
    constexpr auto gcd(M m, N n) -> common_int_t<M, N>
    {
        using CT = common_int_t<M, N>;
        return n == 0 ? abs<CT>(m) : gcd<CT, CT>(n, m % n);
    }
}

template <typename T, typename R = std::ratio<1>>
class denomination
{
public:
    using type  = T;
    using ratio = R; 

private:
    type amount;

public:
    denomination() = default;
    denomination(const denomination&) = default;
    denomination& operator=(const denomination&) = default;
    ~denomination() = default;

    template <typename T2, typename = std::enable_if_t<
        std::is_convertible<T2, type>::value
    >>
    explicit denomination(const T2& t)
        : amount(t)
    {
    }

    template <typename T2, typename R2>
    denomination(const denomination<T2, R2>& d)
        : amount(denomination(
                static_cast<type>(
                    static_cast<std::common_type_t<type, T2, std::intmax_t>>(d.count())
                ) * 
                static_cast<std::common_type_t<type, T2, std::intmax_t>>(
                    std::ratio_divide<R2, ratio>::num
                ) /
                static_cast<std::common_type_t<type, T2, std::intmax_t>>(
                    std::ratio_divide<R2, ratio>::den
                )
            ).count())
    {
    }

    type count() const
    {
        return amount;
    }
};

namespace std
{
    template <typename T1, typename R1, typename T2, typename R2>
    struct common_type<denomination<T1, R1>,
                       denomination<T2, R2>>
    {
        using type = denomination<
            std::common_type_t<T1, T2>, 
            std::ratio<
                utils::gcd(R1::num, R2::num), 
                (R1::den / utils::gcd(R1::den, R2::den)) * R2::den
            >
        >;
    };  
}

template <typename T1, typename R1, typename T2, typename R2>
bool operator==(const denomination<T1, R1>& lhs,
                          const denomination<T2, R2>& rhs)
{
    using CT = typename std::common_type<denomination<T1, R1>,
                                         denomination<T2, R2>>::type;
    return CT(lhs).count() == CT(rhs).count();
}

using dollars = denomination<std::int64_t>;
using cents   = denomination<std::int64_t, std::centi>;

int main()
{
    dollars d{1};
    cents   c{100};
    dollars d2{c};
    dollars d3{2};
    cents   c2{200};
    std::cout << (d == c) 
              << " " 
              << d2.count()
              << " "
              << (d3 == c2);
}
\$\endgroup\$
1
\$\begingroup\$

Value constructor

Well done on making the value constructor explicit. However, you're using the wrong type trait. You're checking if T2 is_convertible to type. But that's not what we're going to do with the T2. We're not implicitly converting. We're explicitly constructing. The two are not synonyms. You should instead prefer:

template <typename T2, typename = std::enable_if_t<
    std::is_constructible<type, const T2&>::value
>>
explicit denomination(const T2& t)
    : amount(t)
{ }

Converting constructor

There's a lot of common_type usage going on here and I don't think you need it. We can introduce the divided ratio as a separate defaulted template argument so that we only have to call it once. Note that you're using common_type to static_cast... and then casting it back to type anyway. There's no need for that intermediate cast. There's definitely no need to cast ::num or ::den. Just use it directly to delegate to the value constructor:

template <typename T2,
          typename R2,
          typename RD = typename std::ratio_divide<R2, R>::type>
denomination(const denomination<T2, R2>& d)
    : denomination(d.count() * RD::num / RD::den)
{ }

You could add SFINAE on that if you want.

Equality

Also overcomplicated with common_type here. Just convert the second type to the first type. If that doesn't work, you won't be able to do this anyway:

template <typename T1, typename R1, typename T2, typename R2>
bool operator==(const denomination<T1, R1>& lhs,
                          const denomination<T2, R2>& rhs)
{
    return lhs.count() == denomination<T1, R1>{rhs}.count();
}

No extra utils functions or partial specializations in namespace std necessary.

Some Checking

The R in denomination<T, R> has to be std::ratio and the T has to be integral, as far as I can tell. It'd be worth just adding some assertions to that effect:

template <typename T> struct is_ratio : std::false_type { };
template <typename N, typename D> struct is_ratio<std::ratio<N, D>> : std::true_type { };

template <typename T, typename R = std::ratio<1>>
class denomination
{
    static_assert(std::is_integral<T>::value, "T must be integral");
    static_assert(is_ratio<R>::value, "R must be a std::ratio");
    ...
};

Exposing count()

Part of the advantage of wrapping values into type safe units like this is to avoid having raw values floating around. Do you really need to expose count()? I'd think twice about it. Just make all denomination<T,U>s friends of each other.

\$\endgroup\$
  • \$\begingroup\$ Can you elaborate more on the first point? Also, thanks for the great advice. \$\endgroup\$ – user85575 Sep 28 '15 at 19:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.