Answering questions in reverse order:
What is the difference between steady_clock vs system_clock in layman
terms.
If you're holding a system_clock in your hand, you would call it a watch, and it would tell you what time it is.
If you're holding a steady_clock in your hand, you would call it a stopwatch, and it would tell you how fast someone ran a lap, but it would not tell you what time it is.
If you had to, you could time someone running a lap with your watch. But if your watch (like mine) periodically talked to another machine (such as the atomic clock in Boulder CO) to correct itself to the current time, it might make minor mistakes in timing that lap. The stopwatch won't make that mistake, but it also can't tell you what the correct current time is.
Does my above code look right?
No. And even if it gave you reasonable answers, I would not say it is right. Don't feel bad, this is a beginner mistake that lots of people make with the <chrono> library.
There is a simple rule I follow with the <chrono> library. The rule is actually not completely correct (thus it is a guideline). But it is close enough to correct to be a guideline that is nearly always followed:
Don't use count().
And a corollary:
Don't use time_since_epoch().
The <chrono> library is designed around a type-safe system meant to protect you from units conversions mistakes. If you accidentally attempt an unsafe conversion, the error is caught at compile time (as opposed to it being a run time error).
The member functions count() and time_since_epoch() are "escape hatches" out of this type-safe system ... to be used only in cases of emergency. Such emergencies arise when (for example) the committee neglects to give you all the tools you need to get the job done (such as I/O) for the <chrono> types, or such as the need to interface with some other timing API via integers.
Review your code and other's for use of count() and time_since_epoch() and scrutinize each use of these functions: Is there any way the code could be rewritten to eliminate their use?
Reviewing the first line of your code:
uint64_t now = duration_cast<milliseconds>(steady_clock::now().time_since_epoch()).count();
now is a time_point (from steady_clock). It units are milliseconds, but at this time I'm not convinced that the units are important. What is important is that now is a time_point retrieved from steady_clock:
auto now = steady_clock::now();
Your second line is more complicated:
bool is_old = (120 * 1000 < (now - data_holder->getTimestamp()));
Let's start with data_holder->getTimestamp(): If you can modify getTimestamp(), you should modify it to return a time_point instead of a uint64_t. To do so, you will have to know the correct units (which you do -- milliseconds), and you will have to know the correct epoch. The epoch is the time point against which your milliseconds are measured from.
In this case 1437520382241ms is about 45.6 years. Assuming this is a recent time stamp, 45.6 years ago was very close to 1970-01-01. As it turns out, every implementation of system_clock() uses 1970-01-01 as its epoch (though each implementation counts different units from this epoch).
So either modify getTimestamp() to return a time_point<system_clock, milliseconds>, or wrap the return of getTimestamp() with time_point<system_clock, milliseconds>:
auto dh_ts = system_clock::time_point{milliseconds{data_holder->getTimestamp()}};
Now your second line is down to:
bool is_old = (120 * 1000 < (now - dh_ts));
Another good guideline:
If you see conversion factors in your <chrono> code, you're doing it wrong. <chrono> lives for doing the conversions for you.
bool is_old = (minutes{2} < (now - dh_ts));
This next step is stylistic, but now your code is simple enough to get rid of your excess parentheses if that is something that appeals to you:
bool is_old = minutes{2} < now - dh_ts;
If you were able to modify the getTimestamp() to return a type-safe value this code could also look like:
bool is_old = minutes{2} < now - data_holder->getTimestamp();
Alas, either way, this still does not compile! The error message should state something along the lines that that there is no valid operator-() between now and dh_ts.
This is the type-safety system coming in to save you from run time errors!
The problem is that time_points from system_clock can't be subtracted from time_points from steady_clock (because the two have different epochs). So you have to switch to:
auto now = system_clock::now();
Putting it all together:
#include <chrono>
#include <cstdint>
#include <memory>
struct DataHolder
{
std::chrono::system_clock::time_point
getTimestamp()
{
using namespace std::chrono;
return system_clock::time_point{milliseconds{1437520382241}};
}
};
int
main()
{
using namespace std;
using namespace std::chrono;
auto data_holder = std::unique_ptr<DataHolder>(new DataHolder);
auto now = system_clock::now();
bool is_old = minutes{2} < now - data_holder->getTimestamp();
}
And in C++14 that last line can be made a little more concise:
bool is_old = 2min < now - data_holder->getTimestamp();
In summary:
- Refuse to use
count() (except for I/O).
- Refuse to use
time_since_epoch() (except for I/O).
- Refuse to use conversion factors (such as 1000).
- Argue with it until it compiles.
If you succeed in the above four points, you will most likely not experience any run time errors (but you will get your fair share of compile time errors).
