Since my initial answer, in which I asked the question "Why bother constructing 2 instances of DateTime
?", markegli has given me a reason why one would do this.
In case the diff between 2 dates is likely to be >= 1 month, using DateTime
is preferable. DateTime
can take things into account like: leap years, variable month length (28-31 days) and, though not applicable to time-stamps, time-zone differences and DST (daylight saving time).
In that case, please ignore the section of my review that recommends dropping the use of DateTime
in favour of constructing a DateDiff
object from the off. Just skip right ahead to the bit about str_replace
, just after this:
A function/method's signature ideally enforces uniformity, and should be regarded as part of that function's documentation.
When working with dates in any programming language, dates can be a pain to deal with. That's precisely why DateTime
exists. It is therefore not too difficult to imagine that the code that ends up calling your function is already using the DateTime
class. As it now stands, your function, then, needs to be called like so:
time_elapsed(
$myDT1->format('Y-m-d H:i:s'),
$myDT2->format('Y-m-d H:i:s')
);
Basically, creating 2 strings from a DateTime
instance, only to create a secondary, temporary instance for both these dates all over... that's just silly.
One alternative, then, might be:
function time_elapsed(DateTime $date1, DateTime $date2)
{
$diff = $date1->diff($date2);
//and so on
}
Now whoever ends up using your function knows what to do: pass 2 DateTime
instances to this function. If he already has these instances, then no harm done... and 2 constructor calls have been made redundant.
If the code isn't yet using the DateTime
class, then it's a pretty easy fix:
time_elapsed(
DateTime::createFromFormat('Y-m-d H:i:s', $dtString1),
DateTime::createFromFormat('Y-m-d H:i:s', $dtString2)
);
Or simply
time_elapsed(
new DateTime($date1),
new DateTime($date2)
);
But now we still have this rather annoying $date1->diff($date2)
call in our function. What if I, as a user want to format a diff I've obtained in a different way? Why wouldn't we change our function to simply accept 1 argument: a DateInterval
instance:
function time_elapsed(DateInterval $diff)
{
//format the diff
}
Now our function serves a clear purpouse: its job is not to compute the diff between 2 dates, its job is simply to format a diff in a given way, and remove unwanted zeroes and plurals.
Now it's just a matter of working on the function some more to allow some more variety:
function time_elapsed(DateInterval $diff, $format = '%y years, %m months, %a days, %h hours, %i minutes, %S seconds')
{
}
If you ever implement this function as a method, you can easily use constants for the format, too. Just play around with this... in the end, you'll just end up calling this function like so:
time_elapsed(
$date1->diff($date2)
);
//or, if you only want the diff in days for some reason:
time_elapsed(
$date1->diff($date2),
'%a days'
);
IMO, this leaves you with a shorter, cleaner and more self-explanatory function that, in some ways is even more flexible. I'm not confined to a single format, nor am I obliged to pass 2 dates to this function for every call.
As an added bonus: people using your function who are as of yet unaware of the DateTime
class will have to educate themselves, and perhaps finally get round to learning to use this class.
At the bottom of my original answer, you'll find a full example of what the function you end up with could look like, including the suggested alternatives to your str_replace
solution.
Why do you bother constructing those two instances of DateTime
? If you're sure you're dealing with 2 timestamps anyway, Why not subtract them, and pass them to the DateInterval
constructor from the off?
function time_elapsed($ts1, $ts2)
{
$diffAbs = abs($ts1 - $ts2);//get absolute value
$diff = new DateInterval(
sprintf('P%dS', $diffAbs)
);
$ret = $diff->format(
'%y years, %m months, %a days, %h hours, %i minutes, %S seconds'
);
Then, instead of using the str_replace
with an array as long as yours, I'd, for once, opt for a regex approach instead:
$ret = preg_replace('/\b0+\s+[a-z]+,?\s*/i', '', $ret);
This changes a string like "1 years, 0 months, 3 days, 5 hours, 12 minutes, 0 seconds" into "1 years, 3 days, 5 hours, 12 minutes"
The expression works like this:
\b0+
: a word boundary (beginning of string, space, dash, slash... whatever) followed by one or more zeroes
\s+
: matches one or more spaces
[a-z]+
: one or more chars
,?
: matches a comma, if there is one
\s*
: zero or more spaces
i
: case-insensitive flag
I wouldn't care about the "1 years" substring, but you can get rid of it using the following expression:
$ret = preg_replace('/(\b1\s+[a-z]+)s\b/i', '$1', $ret);
Which, when applied to "1 years, 3 days, 5 hours, 12 minutes" results in "1 year, 3 days, 5 hours, 12 minutes"
This expression works similarly to the previous one:
(\b1\s+
: matches a word boundary, one 1
char and one or more spaces, this match is grouped
[a-z]+)
: chars are matches (one or more) and is added to the group, so we can use this match in our replacement string
s\b
: matches a literal s (for plural*s*), if it's the end of a word (hence \b
). This match is not grouped
i
: case insensitive
'$1'
: is our replacement, it refers back to the group we make in our pattern: \b1\s+[a-z]
: in the case of "1 days", the entire string will be matched and replaced with the group: "1 day", effectively removing the s at the end
With some work, you'll probably be able to work this into a single expression, using a callback function.
This would leave us with a function that looks like this:
function time_elapsed($ts1, $ts2)
{
if (!is_int($ts1) || !is_int($ts2))
{//add some validation, is_int should do it, since timestamps ARE ints
throw new InvalidArgumentException(__FUNCTION__.' expects 2 ints');
}
$diff = new DateInterval(
sprintf('P%dS', abs($ts1, $ts2))
);
$ret = $diff->format(
'%y years, %m months, %a days, %h hours, %i minutes, %S seconds'
);
return preg_replace(
'/(\b1\s+[a-z]+)s\b/i', '$1',//remove trailing 's'
preg_replace(
'\b0+\s+[a-z]+,?\s*','',//remove 0 diff-values
$ret
)
);
}
The main benefit of regex's here is that the replace calls don't rely on an array that has to match the amount of spaces (trailing or leading), comma's and what have you. Instead, you're free to change the order in which you print out the diff, and the regex will still be able to work out what needs to be removed, and what not.
After my edit, a full example using just the DateInterval
argument, then, would look like this:
function time_elapsed(DateInterval $diff, $format = '%y years, %m months, %a days, %h hours, %i minutes, %S seconds')
{
$ret = $diff->format($format);
return preg_replace(
'/(\b1\s+[a-z]+)s\b/i', '$1',
preg_replace(
'\b0+\s+[a-z]+,?\s*','',
$ret
)
);
}
In this case, it is imperative that you use the preg_replace
, as you no longer fully control the format that is being used.
@
has nothing to do with error reporting (in this context). DateTime can accept a timestamp if you prepend it with a@
. It is not the error suppression operator. And the curly braces -- they serve as a substitution for concatenation.echo "Hello, {$foo}";
is the same asecho "Hello, $foo";
orecho "Hello, " . $foo;
\$\endgroup\$DateTime
class, but do you really need to useDateTime
, and if so, why? Because as I show in my answer, usingDateInterval
(which is whatDateTime::diff
returns anyway) will do just fine \$\endgroup\$