Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sadly VS2012's duration_cast is broken, and I actually need the functionality which is broken. So, I wrote my own:

template<typename ToUnit, typename Rep, typename Period>
ToUnit duration_cast_2(const std::chrono::duration<Rep, Period>& right)
{
    typedef std::ratio_divide<Period, typename ToUnit::period>::type ratio;
    typedef std::common_type<std::common_type<typename ToUnit::rep, Rep>::type, intmax_t>::type common_type;
    return ToUnit(static_cast<typename ToUnit::rep>(static_cast<common_type>(right.count()) * static_cast<common_type>(ratio::num) / static_cast<common_type>(ratio::den)));
}

But I'm not entirely confident that it's correct.

share|improve this question
    
Links don;t work for me. What is a duration_cast<> supposed to do? –  Loki Astari Feb 18 '13 at 16:06
    
@LokiAstari Turn a std::chrono::duration<long long, std::micro> into a std::chrono::duration<double, std::milli>, for example. That conversion cannot be done implicitly, and needs the cast. –  Dave Feb 18 '13 at 16:28

1 Answer 1

I can't say for sure about the internal logic but I can point some problems or things that could be improved in your piece of code:

  • First of all, typename is missing before the names, making your code non-working with some compilers. I would also use using instead of typedef, but that's a mere matter of taste. Updated version:

    using ratio = typename std::ratio_divide<Period, typename ToUnit::period>::type;
    using common_type = std::common_type<typename std::common_type<typename ToUnit::rep, Rep>::type, intmax_t>::type;
    
  • std::common_type is variadic. You don't have to bother with nested std::common_types:

    using common_type = typename std::common_type<typename ToUnit::rep, Rep, intmax_t>::type;
    
  • I know that MSVC has not a full support for constexpr, but duration_cast is supposed to be constexpr (it should work with the November 2013 CTP though):

    template<typename ToUnit, typename Rep, typename Period>
    constexpr ToUnit duration_cast_2(const std::chrono::duration<Rep, Period>& right)
    {
        // ...
    }
    
share|improve this answer
    
This was actually more than a year ago, and at the time I was using VS2010, so limited to not doing anything you mentioned above. I was most interested in the correctness of the internal logic. The whole thing was actually because there was a bug in the VS2010 duration_cast implementation. I don't know if it's fixed yet, probably is. –  Dave Mar 27 at 12:09
    
@Dave From the link in your question, the problem seems to have been solved :) –  Morwenn Mar 27 at 12:18
    
Oh, lol. Thanks. Also I guess I was using 2012 not 2010. I didn't reread my question, it was a long time ago =P –  Dave Mar 27 at 12:25

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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