Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect diff in months #344

Closed
mjpvandenberg opened this issue Apr 9, 2015 · 13 comments
Closed

incorrect diff in months #344

mjpvandenberg opened this issue Apr 9, 2015 · 13 comments

Comments

@mjpvandenberg
Copy link

The difference in months between March 1st and May 1st in Amsterdam is 2 as expected.

$march = Carbon::create(2015, 3, 1, 0, 0, 0, 'Europe/Amsterdam');
$may   = Carbon::create(2015, 5, 1, 0, 0, 0, 'Europe/Amsterdam');
$diff  = $march->diffInMonths($may);
// $diff = 2

However, in London it's 1.

$march = Carbon::create(2015, 3, 1, 0, 0, 0, 'Europe/London');
$may   = Carbon::create(2015, 5, 1, 0, 0, 0, 'Europe/London');
$diff  = $march->diffInMonths($may);
// $diff = 1

Why? This seems like a bug to me.

@mjpvandenberg
Copy link
Author

Sorry, this appears to be a known bug in DateInterval: https://bugs.php.net/bug.php?id=63953

@mjpvandenberg
Copy link
Author

Notice though that this bug has not been fixed in PHP 5.6 (or its backports), contrary to what the bottom post on https://bugs.php.net/bug.php?id=63953 suggests. See http://3v4l.org/uH88b

@kasperhartwich
Copy link

Have the same problem. Have you made a new bug report for DateInterval? Have you done a workaround?

@mjpvandenberg
Copy link
Author

For my use case, the workaround was to simply add an extra day to the later of the two dates: $diff = $someDate->diffInMonths($laterDate->copy()->addDay()) Of course, that won't fix more subtle daylight saving issues with DateInterval.

@kasperhartwich
Copy link

kasperhartwich commented Aug 2, 2015

It's not all months that have this bug. I changed your example a bit:
https://3v4l.org/nvSK7

@kasperhartwich
Copy link

That means adding a day will give an error the other way around.

@mjpvandenberg
Copy link
Author

It's true that not all months have this bug, as it's related to daylight savings.

See http://3v4l.org/MAcKF, where all I changed from your example was to set $d2 to the 2nd of the month. That is equivalent to adding 1 day to the later date, yes? As you can see, now for each month, the month: line says 1.

Not entirely sure what you mean by "the other way around", but it's true that this workaround could calculate a month too many, if the two original dates were already almost another full month apart. Adding 1 extra day to the later one, might just nudge it enough to give you the real full months difference plus 1. For me, however, that's never going to happen, as I'm never comparing just any two Carbons, I'm always constructing Carbons on the 1st of every month (like we are doing on our examples).

@kasperhartwich
Copy link

So you're adding the day in your first Carbon object and then removing it in the result?

Do you know if this is something the php developers is aware of?

@mjpvandenberg
Copy link
Author

Again: $diff = $someDate->diffInMonths($laterDate->copy()->addDay()) I'm not adding the day to $laterDate itself, I'm adding it to a copy which is discarded immediately after this expression because it didn't get assigned to a variable. So $laterDate did not get changed in this expression at all, and I don't need to subtract a day from it afterwards. If that's not what you mean by "removing it in the result" then I don't know what you mean.

@NoelDavies
Copy link

Is there any progress with this apart from adding a day?

http://stackoverflow.com/questions/27732548/carbon-now-time-wrong seems to say it's a configuration issue. @damonjones

@nemesis621
Copy link

nemesis621 commented Dec 6, 2018

There is still a bug in DateTime::diff
https://3v4l.org/sdNMo

https://bugs.php.net/bug.php?id=52480

@kasperhartwich
Copy link

@nemesis621 I ended up overwriting it in my own Date object: https://gist.github.com/kasperhartwich/dbbf787c7a78428cbc5bf09fb4fae8f6

@kylekatarnls
Copy link
Collaborator

I do not recommend to erase methods, it's maintenance troubles (if you or someone else come later to diffInMonths usages, they can be temped to search it in the doc and will get different behavior). You should rather give an other name to this method (and you can use macro rather than extends).

Note that your method does not take $absolute parameter into account and will be slow with big intervals.

To be clear: PHP DateTime diff is completely buggy (in many different ways depending on which version of PHP you use) as soon as you do calculations using a timezone with DST. But it's not a real problem because you should not do that, handle all dates internally with UTC timezone, then change the timezone of your Carbon instance only when and if you need to display this date to the user.

This way you will standardize your dates handling internally (database, back-end code and server can all be location/timezone agnostic) and it will avoid you to write those big work-around extends.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants