Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am trying to see whether a given timestamp is more than two weeks old or not. If it is two weeks old, then print out "Hello" and if it is not more than two weeks old then print out "World".

I have a variable declared as uint64_t which contains the timestamp in milliseconds.

uint64_t data_timestamp;

How do I check whether this timestamp is more than two weeks old and if it is two weeks old, then print out "Hello" otherwise prints out "World".

This is the code I have which gives me the timestamp in milliseconds for two weeks back from the current date, which I then compare to see whether it is two weeks old or not.

#include <ctime>
#include <chrono>
#include <iostream>

int main()
{

    uint64_t data_timestamp = 1406066507000; 

    const auto now = std::chrono::system_clock::now();
    auto twoWeeks = std::chrono::hours(24 * 14);
    auto lastTwoWeeks = now - twoWeeks;

    auto millis = std::chrono::duration_cast<std::chrono::milliseconds>\
                    (lastTwoWeeks.time_since_epoch()).count();

    std::cout << "Time stamp in milliseconds since UNIX epoch start: "\
              << millis << std::endl;

    if (data_timestamp < millis) { 
        std::cout << "Hello"; 
    } else { 
        std::cout << "World"; 
    }

    return 0;
}

I will be running this code on Ubuntu 12.04. What is a better way of doing this? Are there any improvements which I can do it here? In general, I need to check whether data_timestamp is more than two weeks old or not. If it is two weeks old, then print "hello" otherwise prints "world."

Do I even need to use chrono time here? There are 2 * 7 * 24 * 60 * 60 * 1000 = 1,209,600,000 milliseconds in two weeks, so a simple subtraction of that from the current should tell me what is more than 2 weeks old? right?

share|improve this question
    
Of course, it bears mentioning that days are really only always 24 hours in timezones without DST. Depending on where this is being run for, you're going to need to do quite a bit more complicated math, since presumably you want 2 weeks measured in days, not hours... –  Clockwork-Muse Aug 6 at 23:03

2 Answers 2

up vote 3 down vote accepted
  • You're already using <chrono> (and just that), so you don't need <ctime>. Including it anyway won't automatically allow compilation in pre-C++11 (if that was your intent here).

    Also, since you're using uint64_t, you must include <cstdint> as well.

  • Each variable can be const since they're not being modified. This would still be a good idea for ensuring const-correctness, even though the entire scope is within main().

  • The \ characters for line-continuations are useless here; the wrapping still works.

  • Since you're not needing to flush the buffer, std::endl should be replaced with "\n". This may also give you a slight performance boost.

  • The "hello world" output can be shortened to a single-line ternary statement:

    std::cout << ((data_timestamp < millis) ? "Hello" : "World");
    

    Regarding this comparison: my tests give a signed/unsigned mismatch warning. You have already given data_timestamp an unsigned type, so auto must be deducing a signed type for millis based on the function's return type.

    One way to fix this could be by changing data_timestamp to a signed type such as int64_t, though <chrono> may offer more appropriate alternatives to help avoid uncertainty for this.

  • You don't need your own return 0 at the end of main(). The C++ standard should guarantee that the compiler will do this return for you since success is implied.

share|improve this answer
    
thanks for your suggestion. In general what I was after do I even need chrono here? There are 2 * 7 * 24 * 60 * 60 * 1000 = 1,209,600,000 milliseconds in two weeks, so a simple subtraction of that from the current should tell me what is more than 2 weeks old? right? –  Webby Aug 6 at 20:36
1  
@Webby: I think you would still need it if you needed the current time, plus it's still a replacement over std::clock() and such in <ctime>. –  Jamal Aug 6 at 20:44

You could make you code way cleaner by using some C++14 additions: the user-defined literals for std::chrono. Thanks to them, you don't have to care anymore about the underlying type of your duration data, you only have to think about the usual time units:

auto data_timestamp = 1406066507000ms;

Also, thanks to those user-defined literals, you can also simplify the definition of twoWeeks:

auto twoWeeks = 14 * 24h;

In order to do that, you will have to tell that you are using the stadard library user-defined literals for std::chrono since they are not enabled by default. Add this line in your code:

using namespace std::literals::chrono_literals;

Considering your second question about whether you have to use <chrono>: you could probably use the old <ctime> module, but <chrono> is an improvement over <ctime>. Since you need to get the current time, system_clock::now() is as good as you are going to get.

Moreover, while using simple integers could have been sufficient to compute what you need, using the <chrono> types and durations make your intent explicit: when you read your code again, it is obvious that you are dealing with date and time. That aspect alone is really important since anybody reading your code will know what you are doing instead of wondering what do some integers represent.

share|improve this answer
    
Thanks a lot for your suggestion. One thing which I would like to understand is if I need to do the similar stuff which doesn't support C++11, then what will be the best way to do it? As I cannot compile this code in the project which doesn't support C++11? –  Webby Aug 6 at 21:24
    
@Webby: The biggest thing is that you'll have to go from <chrono> to <ctime>. You'll also lose out on auto and user-defined literals, so you'll really have to rely on the compiler to eliminate unsafe comparisons and such. –  Jamal Aug 6 at 21:27
    
@Webby std::chrono and friends are taken from Boost.Chrono that doesn't require C++11 to work. If you don't want to use any other library, use <ctime>. –  Morwenn Aug 6 at 21:27
    
@Jamal Yeah I understand that. I was trying to see what else I can use to write down the similar stuff in the code which is not C++11 portable. –  Webby Aug 6 at 21:39
    
@Morwenn Thanks for suggestion. If possible can you provide an example which uses <ctime> and does the same thing? –  Webby Aug 6 at 21:40

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.