fpagliughi / sockpp

Modern C++ socket library.
BSD 3-Clause "New" or "Revised" License
782 stars 126 forks source link

Made to_timeval work with any timescale #30

Closed snej closed 4 years ago

snej commented 4 years ago

I have some code that needs to convert steady_clock durations to timeval, and that timescale is different from microseconds. So I trivially changed to_timeval() into a template.

fpagliughi commented 4 years ago

Having the template is cool. But we'd want to force the template parameter type to be a Duration of whatever units, not any type.

Check out the stream_socket timeout calls for an example, like:

https://github.com/fpagliughi/sockpp/blob/ddf71320981ad41a214c21f1de136247f733915e/include/sockpp/stream_socket.h#L198-L201

My other pattern for this is typically to make a concrete call using the most sensible time resolution, then make a template that calls the concrete function with the conversion. Usually I do this in a class hierarchy when different platforms need to implement the concrete class in a different way.

This also shown in stream_socket where the timeout template calls the concrete function:

https://github.com/fpagliughi/sockpp/blob/ddf71320981ad41a214c21f1de136247f733915e/include/sockpp/stream_socket.h#L190

So basically, instead of removing the function, I would probably add a template that calls it, and have the template require a Duration, specifically, like:

#if !defined(_WIN32)
template<class Rep, class Period>
timeval to_timeval(const std::chrono::duration<Rep,Period>& to) {
{
    return to_timeval(std::chrono::duration_cast<std::chrono::microseconds>(dur));
}
#endif
fpagliughi commented 4 years ago

BTW, I just stole the Duration call template from the standard library. It has some wait_for calls scattered around like this: https://en.cppreference.com/w/cpp/thread/condition_variable/wait_for

fpagliughi commented 4 years ago

Merged into develop with the changes shown above and some unit tests.

fpagliughi commented 4 years ago

@snej BTW: I'm not sure my way is better than just doing the template by itself. I just put it like this for now, but I want to run some tests to see if there is any difference in performance or code bloat. This function is relatively trivial, but I tend to use this pattern for bigger things, and am curious which way may be better.