Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I am working on an PHP/MySQL application that uses a different timezone from the server it runs on, and neither of the timezones are UTC. I'm converting displayed times to be in the user's timezone, and am wondering whether my approach is sensible. I would love any feedback you might have.

Firstly, the application's current time is written to the output HTML page as a meta value:

<meta name="date" content="<?php echo date('Y-m-d H:i:s') ?>" />

And, every instance of a datetime string is wrapped thus:

<span class="datetime-convert"><?php echo $data->datetime_column ?></span>

Then, after the page is loaded, the following JQuery gets the application time, the user time, figures out the difference between these, and adds that difference to the datetime data:

$("span.datetime-convert").each(function(){
    var serverTime = parseDatetime($("meta[name='date']").attr("content"));
    var userTime = new Date();
    var userOffset = new Date(userTime.getTime() - serverTime.getTime());
    var timeToConvert = parseDatetime($(this).text());
    var convertedTime = new Date(timeToConvert.getTime() + userOffset.getTime());
    $(this).text(convertedTime.toLocaleString());
});
function parseDatetime(value) {
    var a = /^(\d{4})-(\d{2})-(\d{2}) (\d{2}):(\d{2}):(\d{2})$/.exec(value);
    if (a) {
        return new Date(+a[1], +a[2] - 1, +a[3], +a[4], +a[5], +a[6]);
    }
    return null;
}

What am I doing wrong?! :-)

Update 1: I fixed the calculation of userOffset to use getTime().

Update 2: Thanks to Aleksi's feedback, the script now only calculates the offset once, and does so not from the time provided in the meta element, but a TZ offset in seconds. parseDatetime() is as above.

// Get client and server timezone offsets in seconds.
var serverTimezone = $("meta[name='timezoneoffset']").attr("content");
var clientTimezone = new Date().getTimezoneOffset() * -60; // getTimezoneOffset returns (client timezone * -60)
// Get the offset between the server and client, in milliseconds.
var timezoneOffset = (clientTimezone - serverTimezone) * 1000;
$("span.datetime-convert").each(function(){
    var timeToConvert = parseDatetime($(this).text());
    if (timeToConvert) {
        var convertedTime = new Date(timeToConvert.getTime() + timezoneOffset);
        $(this).text(convertedTime.toLocaleString());
    }
});

See also my gist with this code.

share|improve this question
What is going wrong? I made fiddle out of this jsfiddle.net/xXVPC – Aleksi Yrttiaho Jul 19 '11 at 6:02

1 Answer

up vote 2 down vote

I would think it would be easier and more reliable to use timezone offsets rather than exact server time vs. client time. You can get the client-side offset using new Date().getTimezoneOffset() which returns the offset in minutes from UTC. When the timezone is +2 then the offset is -120.

PHP

<meta name="timezoneoffset" content="<?php echo date('P') ?>" />

HTML

<meta name="timezoneoffset" content="+0200" />

JavaScript

var serverTimeZone= $("meta[name='timezoneoffset']").attr("content").substring(0,3);
// getTimezoneOffset returns (client timezone * -60). 
// With timezones the the formula would be (clientTz - serverTz) * 60 * 60000
var timezoneOffset = (new Date().getTimezoneOffset() + (serverTimeZone * 60)) * 60000;
$("span.datetime-convert").each(function(){
    var date = parseDatetime($(this).text());
    if(date) { // If date is not wellformed the script would crash without this guard
      var offsetDate = new Date(date.getTime() + timezoneOffset);
      $(this).text(offsetDate .toLocaleString());
    }
});
function parseDatetime(value) {
    var a = /^(\d{4})-(\d{2})-(\d{2}) (\d{2}):(\d{2}):(\d{2})$/.exec(value);
    if (a) {
        return new Date(+a[1], +a[2] - 1, +a[3], +a[4], +a[5], +a[6]);
    }
    return null;
}

Also there's no need to calculate the offset for each individual date. It can be actually a bit misleading when the offset is slighlty different for each date.

http://jsfiddle.net/xXVPC/1/

More functional: 

var serverTimeZone= $("meta[name='timezoneoffset']").attr("content").substring(0,3);
// getTimezoneOffset returns (client timezone * -60). 
// With timezones the the formula would be (clientTz - serverTz) * 60 * 60000
var timezoneOffset = (new Date().getTimezoneOffset() + (serverTimeZone * 60)) * 60000;
$("span.datetime-convert").each(function(){
    var e = $(this);
    parseDatetime(e.text(), new function(date) {
      var offsetDate = new Date(date.getTime() + timezoneOffset);
      e.text(offsetDate.toLocaleString());
    });
});
function parseDatetime(value, callback) {
    var a = /^(\d{4})-(\d{2})-(\d{2}) (\d{2}):(\d{2}):(\d{2})$/.exec(value);
    if (a) {
        callback(new Date(+a[1], +a[2] - 1, +a[3], +a[4], +a[5], +a[6]));
    }
}
share|improve this answer
All very good points. Thank you very much! I've incorporated them into my project. – Sam Jul 19 '11 at 6:45
I ended up changing the timezoneoffset meta value to be in seconds, to allow for offsets that are not multiples of 1 hour, and avoid the string manipulation. Also, what's your thinking behind adding the callback to parseDatetime? (See my update for my current solution.) Thanks for helping! – Sam Jul 20 '11 at 2:51
The idea is to avoid duplication of the condition if(a) and to get rid of one assignment operation. The first reason is the important one as you may or may not remember to check for nulls and undefined as you go along and use parseDatetime again in some other context of the program or if you happen to reuse the function is some other context altogether. – Aleksi Yrttiaho Jul 20 '11 at 3:08

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.