1

I am trying to see whether my data is 120 second (or 2 minutes) old or not by looking at the timestamp of the data so I have below code as I am using chrono package in C++:

uint64_t now = duration_cast<milliseconds>(steady_clock::now().time_since_epoch()).count();
// check for 2 minutes old data
bool is_old = (120 * 1000 < (now - data_holder->getTimestamp()));   

uint64_t value = now;
while (now < data_holder->getTimestamp() + 80 * 1000 
        && now < value + 80 * 1000) {
    std::this_thread::sleep_for(std::chrono::milliseconds(100));
    now = duration_cast<milliseconds>(steady_clock::now().time_since_epoch()).count();
}

In the above code data_holder->getTimestamp() is uint64_t which returns timestamp in milliseconds.

Now when I print out now variable value, I see this 10011360 and when I print out data_holder->getTimestamp() value which is 1437520382241 

2015-07-21 16:13:02,530 WARN 0x7f35312d1700 data_check - now value: 10011360 , data holder timestamp: 1437520382241

And from the above data holder timestamp, it doesn't look to be 120 second old data right so I feel something is wrong in my code? Since if I convert that data holder timestamp to actual time (using epoch converter) and then compare it with logs time as shown above it is almost same.

So I decided to use system_clock instead of steady_clock and came up with below code in which I started to use auto instead of uint64_t.

Solution A:

auto now = system_clock::now();
auto dh_ts = system_clock::time_point{milliseconds{data_holder->getTimestamp()}};
bool is_old = (minutes{2} < (now - dh_ts));

Earlier, I was using now variable value as uint64_t instead of auto. Now after the above code, I have something like this in my original code since now is not uint64_t so I am getting compilation error while compiling the code.

uint64_t value = now;
while (now < data_holder->getTimestamp() + 80 * 1000 
        && now < value + 80 * 1000) {
    std::this_thread::sleep_for(std::chrono::milliseconds(100));
    now = duration_cast<milliseconds>(steady_clock::now().time_since_epoch()).count();
}

What is the right way to fix this? I cannot change data_holder->getTimestamp() data type, it has to be uint64_t since other code is also using it.

Here is the error:

error: cannot convert std::chrono::time_point<std::chrono::_V2::system_clock, std::chrono::duration<long int, std::ratio<1l, 1000000000l> > >â to âuint64_t {aka long unsigned int}â in initialization

UPDATE:

Can I use like this instead of using Solution A if everything looks good below?

Solution B:

uint64_t now = duration_cast<milliseconds>(system_clock::now().time_since_epoch()).count();
bool is_old = (120 * 1000 < (now - data_holder->getTimestamp()));
user1950349
  • 4,738
  • 19
  • 67
  • 119
  • so, your `while` loop condition consists of 8 operators and a function call. And it involves magic numbers. Complex code that works is marginally ok: complex code that doesn't work should be made less complex. (and no, I don't care if that particular line works, its existence detracts from debugging the rest of the nearby code). I don't see where you set `getTimeStamp`'s data -- what epoch is it using, and how do you know it is the same one as the clock you are using? – Yakk - Adam Nevraumont Jul 23 '15 at 00:06

2 Answers2

2

At least as I read it, this is sleeping 100 ms at a time, then checking whether it's slept for 2 minutes yet. Then repeating until it reaches the 2 minute point.

It seems to me like it makes a lot more sense to compute the desired time, and sleep until then:

struct foo { 
    time_point<system_clock> time_stamp;

    time_point<system_clock> get_timestamp() { return time_stamp; }
    foo() : time_stamp(system_clock::now()) {}
};

// ...    
foo f;

std::this_thread::sleep_until(f.get_timestamp() + 2m);

This does use the (new in C++14) user defined literal to construct a duration of 2 minutes. If you really need to support an older (C++11) compiler, you'll need to use minutes(2) instead.

As far as the title question goes, I'd say: just say no. Far better to store your time_points as actual time_points than insist on stuffing them into integers, then turn them back into time_points when you need to use them again. It's not at all apparent that this accomplishes anything useful in exchange for the pain.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
0

Actually, I would suggest going more in the direction of solution A and converting the rest of your uint64_t times into time_points: the chrono unit system is pretty useful. I would start by defining a helper function to convert from the uint64_t timestamps on your objects to time_points:

using u64_millis = duration<uint64_t, milli>;
static time_point<system_clock, u64_millis> u64_to_time(uint64_t timestamp) {
    return time_point<system_clock, u64_millis>{u64_millis{timestamp}};
}

if your epoch is different than the one for system_clock, this would be the place to fix it. It would probably also work to use milliseconds instead of u64_millis, but the representation type for milliseconds isn't explicitly defined and doing it the above way ensures that the types match up correctly.

Now, the code that you posted becomes something like:

auto now = system_clock::now();
bool is_old = now - u64_to_time(data_holder->getTimestamp()) > minutes{2};

auto value = now;
while (now - u64_to_time(data_holder->getTimestamp()) < seconds{80}
        && now - value < seconds{80}) {
    this_thread::sleep_for(milliseconds{100});
    now = system_clock::now();
}
hdastwb
  • 149
  • 2