4
\$\begingroup\$

I have this function which calculates how many seconds, mins, hours etc ago a specific time string was.

However, the way I have done it consists of a massive if statement block, and it goes through each specific time that it can reach.

Is there a better way of writing this code where it's more efficient and doesn't have a massive if statement block?

define('SECS_IN_MIN', 60);
define('SECS_IN_HR', 3600);
define('SECS_IN_DAY', 86400);
define('SECS_IN_WEEK', 604800);   // 7 days
define('SECS_IN_MONTH', 2592000); // 30 days
define('SECS_IN_YEAR', 31536000); // 365 days

// Determines if a number is a plural
// Input: Takes in a number
// Output: Outputs a string which is either "s", if plural
//         OR "" if not plural
function is_plural($num) {
    if ($num != 1) return "s";
    else return "";
}

// Gets a string line x minutes ago
// Input a date
// Output: a string which matches its time ago
function date_to_str($time) {
    $currTime = time();
    $pastTime = strtotime($time);
    $diff = $currTime - $pastTime;

    // Seconds Ago
    if ($diff < SECS_IN_MIN) {
        $plural = is_plural($diff);
        return "$ans few second$plural ago";
    
    // Minutes Ago
    } else if($diff < SECS_IN_HR) {
        $ans = floor($diff / SECS_IN_MIN);
        $plural = is_plural($ans);
        return "$ans minute$plural ago";
    
    // Hours Ago
    } else if ($diff < SECS_IN_DAY) {
        $ans = floor($diff / SECS_IN_HR);
        $plural = is_plural($ans);
        return "$ans hour$plural ago";
    
    // Days Ago
    } else if ($diff < SECS_IN_WEEK) {
        $ans = floor($diff / SECS_IN_DAY);
        $plural = is_plural($ans);
        return "$ans day$plural ago";

    // Weeks ago
    } else if ($diff < SECS_IN_MONTH) {
        $ans = floor($diff / SECS_IN_WEEK);
        $plural = is_plural($ans);
        return "$ans week$plural ago";

    // Months Ago
    } else if ($diff < SECS_IN_YEAR) {
        $ans = floor($diff / SECS_IN_MONTH);
        $plural = is_plural($ans);
        return "$ans month$plural ago";
    
    // Years Ago
    } else if ($diff > SECS_IN_YEAR * 10) {
        $ans = floor($diff / SECS_IN_YEAR);
        $plural = is_plural($ans);
        return "$ans year$plural ago";
    }

    return "-1";
}

$time = "2020-08-25 13:02:32"
echo( date_to_str($time) );
// Outputs 4 days ago
\$\endgroup\$
1
  • \$\begingroup\$ We must not extend the functionality of your code in our review. We are only supposed to offer insights on how to refine your working code. After having a red hot go yourself, if you are unable to extend the functionality as required, then you should probably ask on Stack Overflow. The title of your question must uniquely describe what your script does -- not describe your concerns for the code. \$\endgroup\$ Aug 29 '20 at 5:14
2
\$\begingroup\$

First, here is my recommended replacement for your script (inspired by this SO post) and some test cases:

Code: (Demo)

function secondsToTime($seconds) {
    $dtF = new DateTime('@0');
    $dtT = new DateTime("@$seconds");
    $diff = $dtF->diff($dtT);
    $units = [
        'y' => 'year',
        'm' => 'month',
        'd' => 'day',
        'h' => 'hour',
        'i' => 'minute',
        's' => 'second'
    ];
    foreach ($units as $char => $unit) {
        if ($diff->$char) {
            if ($char === 'd' && $diff->$char >= 7) {
                $diff->$char = floor($diff->$char / 7);
                $unit = 'week';
            }
            return sprintf(
                '%d %s%s ago',
                $diff->$char,
                $unit,
                $diff->$char !== 1 ? 's' : ''
            );
        }
    }
}

$tests = [
    53 =>  53,           // y=0, m=0, d=0,  h=0, i=0,  s=53
    365 => 365,          // y=0, m=0, d=0,  h=0, i=6,  s=5
    7200 => 7200,        // y=0, m=0, d=0,  h=2, i=0,  s=0
    176455 => 176455,    // y=0, m=0, d=2,  h=1, i=0,  s=55
    2002000 => 2002000,  // y=0, m=0, d=23, h=4, i=6,  s=40
    4592000 => 4592000,  // y=0, m=1, d=22, h=3, i=33, s=20
    66536000 => 66536000 // y=2, m=1, d=9,  h=2, i=13, s=20
];

var_export(
    array_map('secondsToTime', $tests)
);

Output:

array (
  53 => '53 seconds ago',
  365 => '6 minutes ago',
  7200 => '2 hours ago',
  176455 => '2 days ago',
  2002000 => '3 weeks ago',
  4592000 => '1 month ago',
  66536000 => '2 years ago',
)

By leveraging the beauty of a PHP DateTime object, you can pick out the largest non-zero unit from the diff() evaluation and do a quick return from the loop/function. As a bonus, you won't need to worry about daylight savings or leap years or anything -- trust the diff() method.

If you call var_export($diff), you will see that there are additional properties populated, but you only need 6 of them.

I don't think I'd bother with the overhead of a pluralizing function, just write the condition as an inline condition (ternary).

If you want to make $unit a constant, okay, because it won't be mutated.

The only special handling that is necessary is the week calculation because that is not an instantly available property in the object. Do the simple arithmetic and change the unit to be presented and proceed with returning the desired string.


As for critiquing your code:

  • $ans is always returned but it is not always declared; I assuming this is a typo that you overlooked.
  • Your is_plural() condition block is missing its curly braces.
  • else if as two words in php is a violation of PSR-12 coding standards. You should form one word as elseif.
  • I hope that your -1 return is never reached, but if it does then that would suggest that you need to validate your incoming seconds value.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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