Reputation: 12371
I'm trying to update the basic dev library of my project from C++98 to C++11.
In the dev library, there are many functions about time, such as
uint64_t getCurrentMSTime()
{
struct timeval stv;
gettimeofday(&stv, NULL);
uint64_t ms = stv.tv_sec ;
ms = ms * 1000 + stv.tv_usec / 1000;
return ms;
}
I'm trying to change it with std::chrono
of C++11.
For now it seems that I have two choices, one is to return time_point
, the other is to return immediately std::chrono::milliseconds::rep
std::chrono::time_point<std::chrono::system_clock> getCurrentTime1() {
return std::chrono::system_clock::now();
}
std::chrono::milliseconds::rep getCurrentTime2() {
return std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
}
Well, I know that the first one is more flexible because it returns a time_point
, which means that we can convert it into milliseconds, nanoseconds, seconds etc, whereas the second one is return only milliseconds.
But let's say that the developers use ONLY milliseconds so we can ignore the flexible issue.
In this case, which one is better?
BTW, the developers would do something like this: std::map<std::string, ???> mp;
. So my code will decide the ???
part.
std::map<std::string, std::chrono::time_point<std::chrono::system_clock>> mp{std::string("abc"), getCurrentTime1()};
vs
std::map<std::string, std::milliseconds::rep> mp{std::string("abc"), getCurrentTime2()};
.
Which one is better? Or are they almost the same?
Upvotes: 3
Views: 1309
Reputation: 218700
I agree with the currently accepted answer that you should value type-safety, and not return an integral type. However I disagree that returning milliseconds
is best.
Type safety applies to the difference between time points and time durations as well. For example it makes perfect sense to add two time durations. But it is nonsensical to add two time points, though you can subtract them yielding a time duration.
Since the meaning of getCurrentTime()
is to return the current point in time, one should return a std::chrono::time_point
. One can easily choose to return a time_point
based on system_clock
with milliseconds
precision:
std::chrono::time_point<std::chrono::system_clock, std::chrono::milliseconds>
getCurrentTime()
{
return std::chrono::time_point_cast<std::chrono::milliseconds>(
std::chrono::system_clock::now());
}
In C++20, there's a convenience type alias for this type to make it a little easier to spell. You could create such a type alias for yourself if you would like to get a head start on things:
std::chrono::sys_time<std::chrono::milliseconds>
getCurrentTime()
{
return std::chrono::time_point_cast<std::chrono::milliseconds>(
std::chrono::system_clock::now());
}
Or you can create an even shorter name for use within your application, perhaps:
using MSTime = std::chrono::time_point<std::chrono::system_clock, std::chrono::milliseconds>;
...
MSTime
getCurrentMSTime()
{
return std::chrono::time_point_cast<MSTime::duration>(
std::chrono::system_clock::now());
}
Upvotes: 4
Reputation: 38267
You certainly don't want to throw away the type information that <chrono>
offers. If you convert the time_point
you get from the system clock into an integral value right away, you can equally well keep the legacy function as it is.
Instead, decide upfront whether you want to handle points in time relative to the epoch or not. The old function suggests that you want this, so your function should look like this;
std::chrono::milliseconds getDurationSinceEpoch()
{
return std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::system_clock::now().time_since_epoch());
}
But let's say that the developers use ONLY milliseconds so we can ignore the flexible issue
Don't. What you get is here is for free. Whenever this requirement changes, and some developer mixes getDurationSinceEpoch()
return values with durations in whatever resolution, it becomes quite brittle. By baking the correct units into your functions, you guard against future bugs.
Last, you want your map to have this signature:
std::map<std::string, std::chrono::milliseconds>
Upvotes: 1