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

[9.x] Add Conditionable Trait to Illuminate\Support\Carbon #42500

Merged

Conversation

ralphjsmit
Copy link
Contributor

@ralphjsmit ralphjsmit commented May 24, 2022

Today I had a situation where I needed to set the week-number of a Carbon date, but only when a user selected a filter. Otherwise the Carbon instance should default to the current time.

I think that many developers as well have had the situation where they wanted to conditionably apply a method to a Carbon object. At least, I've had this feeling multiple times previously.

I could've done this with an if-condition, but I really missed the elegance of using the ->when() method from the Conditionable, so this looked like the perfect situation for adding the Conditionable trait.

Simple example

// Could be `null` or `[$year, $week]`
$filter = /** */

// Old:
$carbon = $filter ? Carbon::now()->setIsoDate(...$filter) : Carbon::now();

// New:
$carbon = Carbon::now()->when($filter, fn(Carbon $carbon) => $carbon->setIsoDate(...$filter));

I hope this will be useful to many developers!

@DarkGhostHunter
Copy link
Contributor

I was just gonna make this PR...

@driesvints
Copy link
Member

I actually think the old way is so much cleaner here 😅

@ralphjsmit
Copy link
Contributor Author

ralphjsmit commented May 25, 2022

I actually think the old way is so much cleaner here 😅

😅 There might be better examples, where it's more of an improvement. This is just rather simplified, as everyone knows how the Conditionable works :)

How about this?

$carbon = Carbon::now()
    ->when($filter, fn (Carbon $carbon) => $carbon->setIsoDate(...$filter))
    ->startOfWeek();

// Versus
$carbon = $filter
    ? Carbon::now()->setIsoDate(...$filter)->startOfWeek()
    : Carbon::now()->startOfWeek();

// Or ...
$carbon = Carbon::now()
    ->when($filter)->setIsoDate(...$filter)
    ->startOfWeek();

It's just that I prefer closures over proxies, because it's annoying to lose autocompletion.

@taylorotwell taylorotwell merged commit f9fda99 into laravel:9.x May 25, 2022
chu121su12 pushed a commit to chu121su12/framework that referenced this pull request May 26, 2022
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

Successfully merging this pull request may close these issues.

None yet

4 participants