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

BUG on addMonth #923

Closed
minkbear opened this issue Mar 29, 2017 · 13 comments
Closed

BUG on addMonth #923

minkbear opened this issue Mar 29, 2017 · 13 comments

Comments

@minkbear
Copy link

minkbear commented Mar 29, 2017

if Today = 2017-03-29
and Today - 1 month = should be 2017-02-28 (expected end of last month)

Here is code

dd(Carbon::now())

Carbon\Carbon {#1352
  +"date": "2017-03-29 13:22:21.000000"
  +"timezone_type": 3
  +"timezone": "Asia/Bangkok"
}

dd(Carbon::now()->addMonth(-1))

Carbon\Carbon {#1352
  +"date": "2017-03-01 13:23:31.000000"
  +"timezone_type": 3
  +"timezone": "Asia/Bangkok"
}
@minkbear minkbear changed the title addMonth BUG BUG on addMonth Mar 29, 2017
@bramslob
Copy link

bramslob commented Mar 29, 2017

Same here, but with Carbon::now()->subMonth(1)->startOfMonth() returning :

Carbon\Carbon {#1341
     +"date": "2017-03-01 00:00:00.000000",
     +"timezone_type": 3,
     +"timezone": "Europe/Amsterdam",
}

@HercDotMe
Copy link

If you can disregard the day and need to check only months and years you can do: Carbon::now()->startOfMonth()->subMonth(1) which will return the first day of last month.

@bramslob
Copy link

@VladErc sadly I do need to work with the day. But as you can see in my comment, I did do the startOfMonth() but that still returned march

@HercDotMe
Copy link

HercDotMe commented Mar 29, 2017

@bramslob You did it the other way (first subMonth then startOfMonth), which, due to the bug, would've returned 1st of March and then take first of that month. My way gets you to first of March and then subtracting a month which will return 1st of February.

@bramslob
Copy link

@VladErc I see yes, that does work. But I do believe this is unintended behavior. The order in this should not matter. But thanks for showing a way in which it works for now

@leepownall
Copy link

#639
#761

Carbon::now()->subMonth() -> Carbon::now()->subMonthNoOverflow()
Carbon::now()->addMonth() -> Carbon::now()->addMonthNoOverflow()

Had same issue, not sure what the underlying reason is, but here is the fix.

@bramslob
Copy link

@leepownall found that option in the source as well, but did not see the explanation of 'overflow'. Now that you pointed to those issues I get it a lot more. Thanks, I guess we'll change our code accordingly

@HercDotMe
Copy link

@leepownall I would've expected default behaviour to be with no overflow, thanks for the fix however!

@DanielDarrenJones
Copy link

I also feel like the default behaviour should be changed to include the overflow, I was not aware of this until I saw a tweet about it just now and this would have certainly caught me out.

@dereuromark
Copy link
Contributor

@DanielDarrenJones https://github.com/cakephp/chronos has the default behavior built in as expected.
For Carbon it would require a major bump in version.

@DanielDarrenJones
Copy link

@dereuromark ooo thanks for sharing that, it seems to cover some of the shortfalls I didn't like with carbon such as immutability, will defo give that a go 😄

@lucasmichot
Copy link
Collaborator

@minkbear then use addMonthsNoOverflow

@lucasmichot
Copy link
Collaborator

And have a look at \Carbon\Carbon::useMonthsOverflow

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

7 participants