2
\$\begingroup\$

I have this function which returns the time that a Tweet was created based on the created_at value that comes when a timeline is pulled. I also check whether or not the tweet was created more than 2 hours ago, and if it was, have an action execute. I was wondering, is there a way that this code could be shortened down because it currently feels very long. I thought about using a switch statement instead of all the ifs, but it didn't really shorten the function down at all.


function timeAgo(dateString) {
   var rightNow = new Date();
   var then = new Date(dateString);
   if ($.browser.msie) {
      then = Date.parse(dateString.replace(/( \+)/, ' UTC$1'));
   }
   var diff = rightNow - then;
   var second = 1000;
   var minute = second * 60;
   var hour = minute * 60;
   var day = hour * 24;
   var week = day * 7;
   if (isNaN(diff) || diff < 0) {
      return "";
   }
   if (diff < second * 2) {
      return {tAgo: "right now", timeCheck: false};
   }
   if (diff < minute) {
      return {tAgo: Math.floor(diff / second) + " seconds ago", timeCheck: false};
   }
   if (diff < minute * 2) {
      return {tAgo: "about 1 minute ago", timeCheck: false};
   }
   if (diff < hour) {
      return {tAgo: Math.floor(diff / minute) + " minutes ago", timeCheck: false};
   }
   if (diff < hour * 2) {
      return {tAgo: "about 1 hour ago", timeCheck: false};
   }
   if (diff < day) {
      return {tAgo: Math.floor(diff / hour) + " hours ago", timeCheck: true};
   }
   if (diff > day && diff < day * 2) {
      return {tAgo: "yesterday", timeCheck: true};
   }
   if (diff < day * 365) {
      return {tAgo: Math.floor(diff / day) + " days ago", timeCheck: true};
   }
   else {
      return {tAgo: "over a year ago", timeCheck: true};
   }
}
\$\endgroup\$
1
  • \$\begingroup\$ $.browser is deprecated - is there someway to do that with bug-detection? \$\endgroup\$
    – Inkbug
    Commented Jul 18, 2012 at 13:38

4 Answers 4

2
\$\begingroup\$

Here is how i'd approach it

// Make all those units constants
var SECOND = 1000;
var MINUTE = SECOND * 60;
var HOUR = MINUTE * 60;
var DAY = HOUR * 24;
var YEAR = DAY * 365;

// store the string variations in a table, seperate from the code
TIMES = [
    {min: 0, text: "right now", unit: SECOND},
    {min: 2 * SECOND, text: "@ seconds ago", unit: SECOND},
    {min: MINUTE, text: "about 1 minute ago", unit: MINUTE},
    {min: 2 * MINUTE, text: "@ minutes ago", unit: MINUTE},
    {min: HOUR, text: "about 1 hour ago", unit: HOUR},
    {min: 2 * HOUR, text: "@ hours ago", unit: HOUR},
    {min: DAY, text: "yesterday", unit: DAY},
    {min: 2 * DAY, text: "@ days ago", unit: DAY},
    {min: YEAR, text: "over a year ago", unit: YEAR}
];

// use the table to generate the date
function timeAgoText(diff) {
    var use;
    for(var index = 0, length = TIMES.length; index < length; index++)
    {
        if( TIMES[index].min < diff )
        {
            use = TIMES[index];
        }
    }
    units = Math.floor(diff / use.unit)
    return use.text.replace("@", units)
}

function timeAgo(dateText)
{
   var then = new Date(dateString);
   if ($.browser.msie) {
      then = Date.parse(dateString.replace(/( \+)/, ' UTC$1'));
   }
   var diff = new Date() - then;
   if( isNaN(diff) || diff < 0 )
   {
       return ""
   }
   else
   {
       // seperate the time check and the text generation
       return {
           tAgo: timeAgoText(diff),
           timeCheck: diff > HOUR * 2 
       }
   }
}
\$\endgroup\$
1
  • \$\begingroup\$ +1, Indeed, this calls out for a table based lookup method. Much simpler to understand and modify then a switch (if/else) based approach. Code complete has further examples of this coding style. \$\endgroup\$ Commented Jul 17, 2012 at 18:59
1
\$\begingroup\$

This doesn't really shorten it, but I think it reads better if you get rid of the diff variable and instead calculate the diff in terms of seconds, minutes, hours and days:

function timeAgo(dateString) {
    var rightNow = new Date(), then = new Date(dateString), seconds, minutes, hours, days;
    if ($.browser.msie) {
        then = Date.parse(dateString.replace(/( \+)/, ' UTC$1'));
    }
    seconds = (rightNow - then) / 1000;
    if (isNaN(seconds) || seconds < 0) {
        return "";
    }
    if (seconds < 2) {
        return {tAgo: "right now", timeCheck: false};
    }
    if (seconds < 60) {
        return {tAgo: Math.floor(seconds) + " seconds ago", timeCheck: false};
    }

    minutes = seconds / 60;
    if (minutes < 2) {
        return {tAgo: "about 1 minute ago", timeCheck: false};
    }
    if (minutes < 60) {
        return {tAgo: Math.floor(minutes) + " minutes ago", timeCheck: false};
    }

    hours = minutes / 60;
    if (hours < 2) {
        return {tAgo: "about 1 hour ago", timeCheck: false};
    }
    if (hours < 24) {
        return {tAgo: Math.floor(hours) + " hours ago", timeCheck: true};
    }

    days = hours * 24;
    if (days < 2) {
        return {tAgo: "yesterday", timeCheck: true};
    }
    if (days < 365) {
        return {tAgo: Math.floor(days) + " days ago", timeCheck: true};
    }

    return {tAgo: "over a year ago", timeCheck: true};
}
\$\endgroup\$
1
\$\begingroup\$

Taking what Bill Barry wrote - I then cleaned up the function a bit, got rid of a few declarations and returns, added error checking for future dates and a couple of test cases. Hope this is something along the lines of what you were looking for.

function timeAgo(dateString) {
    /* removed rightNow, replaced with function call in seconds declaration */
    /* removed seconds, minutes, hours, days declarations, declared below on set */
    /* added new returnVal variable to remove all returns below with 1 return */
    var returnVal, then = new Date(dateString);

    /* left alone */
    if($.browser.msie) then = Date.parse(dateString.replace(/( \+)/, ' UTC$1'));

    /* replaced rightNow in seconds declare with new Date() */
    /* declared variables in place instead of above */
    var seconds = (new Date() - then) / 1000;
    var minutes = seconds / 60;
    var hours = minutes / 60;
    var days = hours * 24;

    /* Check for negative value of seconds (negative indicates a future date) */
    if(seconds < 0) return "";

    /* replaced if statements with ternary operations - might not save much performance wise, but looks cleaner */
    seconds < 2 ? returnVal = {tAgo: "right now", timeCheck: false} :
        seconds < 60 ? returnVal =  {tAgo: Math.floor(seconds) + " seconds ago", timeCheck: false} :
            returnVal =  "";

    minutes < 2 ? returnVal =  {tAgo: "about 1 minute ago", timeCheck: false} : 
        minutes < 60 ? returnVal =  {tAgo: Math.floor(minutes) + " minutes ago", timeCheck: false} : 
            returnVal =  {tAgo: "over a year ago", timeCheck: true};

    hours < 2 ? returnVal =  {tAgo: "about 1 hour ago", timeCheck: false} :
        hours < 24 ? returnVal =  {tAgo: Math.floor(hours) + " hours ago", timeCheck: true} :
            returnVal =  {tAgo: "over a year ago", timeCheck: true};

    days < 2 ? returnVal =  {tAgo: "yesterday", timeCheck: true} :
        days < 365 ? returnVal =  {tAgo: Math.floor(days) + " days ago", timeCheck: true} :
            returnVal =  {tAgo: "over a year ago", timeCheck: true};

        /* Remove JSON.stringify() if you want to return an object */
    return JSON.stringify(returnVal);
}

function testTimeAgo(){
    document.write("For ~Tue Apr 07 22:52:51 +0000 2009~:: " + timeAgo("Tue Apr 07 22:52:51 +0000 2009") + "<br>");
    document.write("For ~Tue July 16 22:52:51 +0000 2012~:: " + timeAgo("Tue July 16 22:52:51 +0000 2012") + "<br>");
    document.write("For ~Tue Apr 07 22:52:51 +0000 2013~:: " + timeAgo("Tue Apr 07 22:52:51 +0000 2013") + "<br>");
    document.write("For ~Tue June 07 22:52:51 +0000 2012~:: " + timeAgo("Tue June 07 22:52:51 +0000 2012") + "<br>");
}

/* Run a few tests */
testTimeAgo();

Without comments or test case, and a couple more improvements:

function timeAgo(dateString) {
    var returnVal, then;

    $.browser.msie ? then = Date.parse(dateString.replace(/( \+)/, ' UTC$1')) : then = new Date(dateString);

    var seconds = new Date() - then) / 1000;
    var minutes = seconds / 60;
    var hours = minutes / 60;
    var days = hours * 24;

    if(seconds < 0) return "";

    seconds < 2 ? returnVal = {tAgo: "right now", timeCheck: false} :
        seconds < 60 ? returnVal =  {tAgo: Math.floor(seconds) + " seconds ago", timeCheck: false} :
            returnVal =  "";

    minutes < 2 ? returnVal =  {tAgo: "about 1 minute ago", timeCheck: false} : 
        minutes < 60 ? returnVal =  {tAgo: Math.floor(minutes) + " minutes ago", timeCheck: false} : 
            returnVal =  {tAgo: "over a year ago", timeCheck: true};

    hours < 2 ? returnVal =  {tAgo: "about 1 hour ago", timeCheck: false} :
        hours < 24 ? returnVal =  {tAgo: Math.floor(hours) + " hours ago", timeCheck: true} :
            returnVal =  {tAgo: "over a year ago", timeCheck: true};

    days < 2 ? returnVal =  {tAgo: "yesterday", timeCheck: true} :
        days < 365 ? returnVal =  {tAgo: Math.floor(days) + " days ago", timeCheck: true} :
            returnVal =  {tAgo: "over a year ago", timeCheck: true};

    return returnVal;
}
\$\endgroup\$
2
  • \$\begingroup\$ Could I initially define returnVal as {tAgo: "over a year ago", timeCheck: true}; so that I don't have it repeat three times as an else return? \$\endgroup\$
    – ayyp
    Commented Jul 16, 2012 at 19:24
  • \$\begingroup\$ You could initially define it as well, and just replace the definitions in else as an empty string (: "";) - remember not to reassign. \$\endgroup\$
    – jsanc623
    Commented Jul 16, 2012 at 19:55
0
\$\begingroup\$

Two words: Use functions.

/** @const */
var ONE_SECOND = 1000;
/** @const */
var ONE_MINUTE = ONE_SECOND * 60;
/** @const */
var ONE_HOUR = ONE_MINUTE * 60;
/** @const */
var ONE_DAY = ONE_HOUR * 24;
/** @const */
var ONE_WEEK = ONE_DAY * 7;
/** @const */
var ONE_YEAR = ONE_DAY * 365;

function createSingularOrPlural(singular, plural, value, divisor) {
    value = Math.floor(value / divisor);

    return value < 2 ? singular : plural.replace('?', value);
}

function getPrettyTimeAgo(timeAgo) {

    if (timeAgo >= ONE_YEAR) {
        return "over a year ago";
    }

    if (timeAgo >= ONE_DAY) {
        return createSingularOrPlural("yesterday", "? days ago", timeAgo, ONE_DAY);
    }

    if (timeAgo >= ONE_HOUR) {
        return createSingularOrPlural("about 1 hour ago", "? hours ago", timeAgo, ONE_HOUR);
    }

    if (timeAgo >= ONE_MINUTE) {
        return createSingularOrPlural("about 1 minute ago", "? minutes ago", timeAgo, ONE_MINUTE);
    }

    return createSingularOrPlural("right now", "? seconds ago", timeAgo, ONE_SECOND);
}

function getMillisecondsSinceDate(dateString) {
    var rightNow = new Date();
    var then = new Date(dateString);

    // If we're using jQuery, check if we're using IE to perform a defect fix. 
    if (window.$ && window.$.browser && window.$.browser.msie) {
        then = Date.parse(dateString.replace(/( \+)/, ' UTC$1'));
    }

    return rightNow - then;
}

function timeAgo(dateString) {

    var timeDifference = getMillisecondsSinceDate(dateString);

    if (isNaN(timeDifference) || timeDifference < 0) {
        return "";
    }

    return {
        tAgo: getPrettyTimeAgo(timeDifference),
        timeCheck: timeDifference >= 2 * ONE_HOUR
    };
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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