views:

50

answers:

4

Guys,

I''m looking to get this correct and i'm getting a bit fustrated with this. What I want to do is get hours and days and weeks correct.

Example:

if this post is < 60min old then have it read: Posted Less then 1 minute ago

if this post is < 120min old then have it read: Posted 1 hour ago

if this post is > 120min old then have it read: Posted 1 hours ago

if this post is < 1440min old then have it read: Posted 1 day ago

if this post is > 1440min old then have it read: Posted 2 days ago

Is that right??

This is what I have so far:

if (lapsedTime < 60) {
        return '< 1 mimute';
    } else if (lapsedTime < (60*60)) {
        return Math.round(lapsedTime / 60) + 'minutes';
    } else if (lapsedTime < (12*60*60)) {
        return Math.round(lapsedTime / 2400) + 'hr';
    } else if (lapsedTime < (24*60*60)) {
        return Math.round(lapsedTime / 3600) + 'hrs';
    } else if (lapsedTime < (7*24*60*60)) {
        return Math.round(lapsedTime / 86400) + 'days';
    } else {
        return Math.round(lapsedTime / 604800) + 'weeks';
    }
A: 

Technically what you have is correct. However, that compound if is a nightmare: not at all easy to understand what it does, or if it works correctly, without thought.

You may want to precalculate all the weeks/days/hours/etc values and then use those to make your code more readable: take a look at my answer here for an example.

In your case, you would do:

var weeks = Math.round(lapsedTime / 604800);
var days = Math.round(lapsedTime / 86400);
// etc for the other quantities

if (weeks >= 1) {
    return weeks + " weeks";
}
else if (days >= 1) {
    return days + " days";
}

// etc

I think you will agree that this is much easier to understand and verify for correctness.

Edit: When scanning your question for obvious problems, I managed to miss the errors all the others have pointed out. What I think this says is: a) I 'm a sloppy quick reader, b) it is indeed hard to verify an if like that. :-)

Jon
+1  A: 

You had a few typos and missed cases:

    if (lapsedTime < 60) {
        return '< 1 minute';
    } else if (lapsedTime < (2*60*60)) {     // Missed this case
        return '1 minute';
    } else if (lapsedTime < (60*60)) {
        return Math.round(lapsedTime / 60) + ' minutes';
    } else if (lapsedTime < (2*60*60)) {     // This should be 2, not 12
        return '1 hour';
    } else if (lapsedTime < (24*60*60)) {
        return Math.round(lapsedTime / 3600) + ' hours';
    } else if (lapsedTime < (2*24*60*60)) {
        return '1 day';
    } else if (lapsedTime < (7*24*60*60)) {
        return Math.round(lapsedTime / 86400) + ' days';
    } else if (lapsedTime < (2*7*24*60*60)) {  // Missed this case
        return '1 week';
    } else {
        return Math.round(lapsedTime / 604800) + ' weeks';
    }

I do agree that a better approach would be to calculate the weeks, days, hours, minutes, and use those to format a string:

function formatTime(t, tStr) {
    // Singular case
    if(t==1) { return t+' '+tStr; }

    // Plural case
    return t+' '+tStr+'s';
}

function timeString(lapsedTime) {
    // These could be "round" or "floor", depending on what you want
    var minutes = Math.floor(lapsedTime/60);
    var hours = Math.floor(lapsedTime/3600);
    var days = Math.floor(lapsedTime/86400);
    var weeks = Math.floor(lapsedTime/604800);
    var years = Math.floor(lapsedTime/31536000);

    if(minutes == 0) {  return '< 1 minute';                    }
    if(hours == 0)   {  return formatTime(minutes, 'minute');   }
    if(days == 0)    {  return formatTime(hours, 'hour');       }
    if(weeks == 0)   {  return formatTime(days, 'day');         }
    if(years == 0)   {  return formatTime(weeks, 'week');       }
    return formatTime(years, 'year');
}
Jeff B
+1  A: 

You don't need to write "elseif" because once returned value, function doesn't execute anymore, so it's safe to write:

if (time < 60) return '< 1 minute';
if (time < 120) return '1 minute';
if (time < 60*60) ...
...

and another mistake is that you make singular of hours, but not of minutes, days and weeks. as i wrote in comment, there is also a typo for one hour, you have 12*60*60 - i think you meant 2*60*60

praksant
A: 

You might find this post useful.

dtryan