10
\$\begingroup\$

Consider these functions that allow to convert std::chrono::time_point to/from std::string with a predefined date-time format.

constexpr size_t log10(size_t xx) { return xx == 1 ? 0 : 1 + log10(xx/10); } template < typename Double, size_t Precision = std::numeric_limits<Double>::digits10, typename TimePoint > requires (TimePoint::period::den % 10 != 0) && std::is_floating_point_v<Double> && Precision <= std::numeric_limits<Double>::digits10 inline bool toString(const TimePoint& timePoint, std::string& str) { Double seconds = timePoint.time_since_epoch().count(); (seconds *= TimePoint::period::num) /= TimePoint::period::den; auto zeconds = std::modf(seconds,&seconds); time_t tt = seconds; std::ostringstream oss; oss << std::put_time(std::localtime(&tt), "%Y-%m-%d %H:%M:") << std::setw(Precision+3) << std::setfill('0') << std::fixed << std::setprecision(Precision) << (tt%60)+zeconds; return oss && (str = oss.str(), true); } template < typename TimePoint > requires (TimePoint::period::den % 10 == 0) inline bool toString(const TimePoint& timePoint, std::string& str) { uint64_t feconds = timePoint.time_since_epoch().count() * TimePoint::period::num; time_t tt = feconds / TimePoint::period::den; std::ostringstream oss; oss << std::put_time(std::localtime(&tt), "%Y-%m-%d %H:%M:%S.") << std::setw(log10(TimePoint::period::den)) << std::setfill('0') << feconds % TimePoint::period::den; return oss && (str = oss.str(), true); } template < typename TimePoint > bool fromString(TimePoint& timePoint, const std::string& str) { std::istringstream iss(str); std::tm tm{}; if (!(iss >> std::get_time(&tm, "%Y-%m-%d %H:%M:%S"))) return false; timePoint = {}; timePoint += std::chrono::seconds(std::mktime(&tm)); if (iss.eof()) return true; if (iss.get() != '.') return false; std::string zz; if (!(iss >> zz)) return false; static_assert(std::chrono::high_resolution_clock::period::num == 1 && std::chrono::high_resolution_clock::period::den % 10 == 0); zz.resize(log10(std::chrono::high_resolution_clock::period::den),'0'); size_t zeconds = 0; try { zeconds = std::stoul(zz); } catch (const std::exception&) { return false; } timePoint += std::chrono::high_resolution_clock::duration(zeconds); return true; } 

with usage:

 std::string str; auto now = std::chrono::system_clock::now(); toString(now,str); std::cerr << "==== " << str << std::endl; using DD = std::chrono::duration<size_t,std::ratio<2,3>>; using TP = std::chrono::time_point<std::chrono::system_clock,DD>; toString<double>(TP(DD(0)),str); std::cout << "==== " << str << std::endl; toString<double>(TP(DD(1)),str); std::cout << "==== " << str << std::endl; toString<double>(TP(DD(2)),str); std::cout << "==== " << str << std::endl; toString<double>(TP(DD(3)),str); std::cout << "==== " << str << std::endl; std::chrono::system_clock::time_point tp; str = "2017-Mar-01"; fromString(tp,str); toString(tp,str); std::cerr << "---- " << str << std::endl; str = "1969-Dec-31 19:00:00.666666666666667"; fromString(tp,str); toString(tp,str); std::cerr << "---- " << str << std::endl; 

Any suggestions to make them faster, c-functions free or more compact?

As the objective is to do conversions with a predefined date-time format the std::locale support of std::ostringstream and std::istringstream is useless and it is a significant slowdown. But how can I avoid this?

\$\endgroup\$
4
  • 3
    \$\begingroup\$Not a full review - but log10() might not be the best choice of name, given that std::log10() exists.\$\endgroup\$CommentedMar 2, 2017 at 15:13
  • \$\begingroup\$@TobySpeight it is usually in the global names. But I believe signature accepts doubles and ints, not unsigned ones. So strong typing might save the user.\$\endgroup\$CommentedMar 2, 2017 at 15:54
  • 1
    \$\begingroup\$for me, it feels like the code is great candidate for a stream. The one that wraps another stream.\$\endgroup\$CommentedMar 2, 2017 at 16:13
  • \$\begingroup\$Is using the sequel to std::chrono considered cheating?\$\endgroup\$CommentedMay 8, 2018 at 14:35

2 Answers 2

8
\$\begingroup\$

Headers and typenames

We're missing

#include <chrono> #include <cmath> #include <cstdint> #include <ctime> #include <iomanip> #include <iostream> #include <sstream> #include <string> 

And we consistently misspell the types std::size_t, std::time_t, std::uint64_t.

A name collision

On systems that define a global log10 as well as std::log10, there's ambiguity on the calls to our log10. I suggest using a name that conveys intent, such as digits() (even that's a bit risky - it would be better in a private namespace). It costs nothing to make it terminate if passed 0 as argument, too:

static constexpr std::size_t feconds_width(std::size_t x) { return x <= 1 ? 0 : 1 + feconds_width(x/10); } 

Interface consistency

The two alternative forms for toString() need to be instantiated differently, depending on the timepoint's denominator value. If we make Double default to double, then both can be called without explicit template arguments.

Incidentally, did you measure a performance difference between the two implementations that proved a benefit? If so, it's worth quantifying that in a comment so that future readers understand that it's worthwhile having both versions.

Document the assumption

This code only works when converting to or from TimePoint classes which have the same epoch as std::time_t.

Prefer to return values over status codes

Rather than returning a status code, I'd prefer to throw an exception when conversion fails; then the return value can be used directly. Failure in toString() seems especially unlikely - I think the only possible cause is running out of room in the output string and being unable to allocate more.

if (!oss) throw std::runtime_error("timepoint-to-string"); return oss.str(); 

If you really dislike exceptions, it's still easier on the caller to return a std::optional rather than return a boolean and modify a reference argument.

Be careful with negative times

If tt<0, then tt%60 can be negative. That's not what we want. However, since we create a std::tm, we can read seconds from that:

auto tm = std::localtime(&tt); if (!tm) throw std::runtime_error(std::strerror(errno)); oss << std::put_time(tm, "%Y-%b-%d %H:%M:") << std::setw(Precision+3) << std::setfill('0') << std::fixed << std::setprecision(Precision) << tm->tm_sec+zeconds; 

Peek the '.' instead of reading it

If we leave '.' in the input stream, we can read that as part of a floating-point value:

double zz; if (iss.peek() != '.' || !(iss >> zz)) throw std::invalid_argument("decimal"); using hr_clock = std::chrono::high_resolution_clock; std::size_t zeconds = zz * hr_clock::period::den / hr_clock::period::num; timePoint += hr_clock::duration(zeconds); 

Test cases seem inconsistent with the code

The test cases use month names, which require a %b conversion, not %m. We could do with a few more test cases.

Consider a streamable wrapper

If your main use-case is to stream dates in and out, it may be better to bypass returning a string, and instead create an object that knows how to stream directly to a std::ostream. As I don't know if that's your intended use, I won't demonstrate that.


Simplified code

#include <cerrno> #include <chrono> #include <cmath> #include <cstdint> #include <ctime> #include <iomanip> #include <iostream> #include <sstream> #include <stdexcept> #include <string> template<typename Double = double, std::size_t Precision = std::numeric_limits<Double>::digits10, typename TimePoint> requires std::is_floating_point_v<Double> && Precision <= std::numeric_limits<Double>::digits10 inline std::string toString(const TimePoint& timePoint) { auto seconds = Double(timePoint.time_since_epoch().count()) * TimePoint::period::num / TimePoint::period::den; auto const zeconds = std::modf(seconds,&seconds); std::time_t tt(seconds); std::ostringstream oss; auto const tm = std::localtime(&tt); if (!tm) throw std::runtime_error(std::strerror(errno)); oss << std::put_time(tm, "%Y-%b-%d %H:%M:") << std::setw(Precision+3) << std::setfill('0') << std::fixed << std::setprecision(Precision) << tm->tm_sec+zeconds; if (!oss) throw std::runtime_error("timepoint-to-string"); return oss.str(); } template<typename TimePoint> TimePoint fromString(const std::string& str) { std::istringstream iss{str}; std::tm tm{}; if (!(iss >> std::get_time(&tm, "%Y-%b-%d %H:%M:%S"))) throw std::invalid_argument("get_time"); TimePoint timePoint{std::chrono::seconds(std::mktime(&tm))}; if (iss.eof()) return timePoint; double zz; if (iss.peek() != '.' || !(iss >> zz)) throw std::invalid_argument("decimal"); using hr_clock = std::chrono::high_resolution_clock; std::size_t zeconds = zz * hr_clock::period::den / hr_clock::period::num; return timePoint += hr_clock::duration(zeconds); } int main() { using std::chrono::system_clock; auto now = system_clock::now(); std::clog << "==== " << toString(now) << std::endl; using DD = std::chrono::duration<std::size_t,std::ratio<2,3>>; using TP = std::chrono::time_point<std::chrono::system_clock,DD>; for (int i = 0; i <= 3; ++i) std::clog << "==== " << toString(TP(DD(i))) << std::endl; for (std::string s: { "2017-May-01 00:10:15", "2017-May-01 00:10:15.25", "2017-Mar-01", "1969-Dec-31 19:00:00.666666666666667", "2018", // underspecified - but succeeds as 2017-12-31 "1752-May-5", // out of range "not a date", }) { try { std::clog << "---- " << toString(fromString<system_clock::time_point>(s)) << std::endl; } catch (const std::exception& e) { std::cerr << e.what() << std::endl; } } } 
\$\endgroup\$
    5
    \$\begingroup\$

    high_resolution_clock

    Standard allows high_resolution_clock to be steady_clock or system_clock or something else. Adding high_resolution_clock to system_clock is only meaningful when high_resolution_clock is an alias of system_clock. We should just use system_clock. There is no need for high resolution as tm is only accurate to seconds.

    system_clock

    System_clock is the only clock that makes sense to be able to convert to std::time_t and chrono library helpfully provided std::chrono::system_clock::to_time_t.

    auto tp_sec = std::chrono::time_point_cast<std::chrono::seconds>(timePoint); auto tt = std::chrono::system_clock::to_time_t (tp_sec); 

    duration_cast

    Instead of converting time_since_epoch().count() to seconds by hand, we can just use auto sec_since_epoch = std::chrono::duration_cast<std::chrono::seconds>o(timePoint.time_since_epoch()); We can also convert to milliseconds ect. Substracting sec from ms since epoch will give us the leftover ms.

    auto tp_ms = std::chrono::time_point_cast<std::chrono::milliseconds>(timePoint); auto ms= (tp_ms - tp_sec).count(); 
    \$\endgroup\$

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.