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

Conditional Clauses #349

Closed
afbora opened this issue Jul 16, 2019 · 12 comments

Comments

@afbora
Copy link

@afbora afbora commented Jul 16, 2019

I will show a feature named Conditional Clauses that I use often in Laravel framework:
https://laravel.com/docs/master/queries#conditional-clauses


Lets continue with a sample:

For ex; i need to filter or sort products by selected filters.

Get request filter params, ok:

$size = $request->get('size');
$color = $request->get('color');
$price = $request->get('price');

I need to write such code to get filtered products, right?

$products = $page->children();

if($size) {
    $products = $products->filterBy('size', $size);
}

if($color) {
    $products = $products->filterBy('color', $color);
}

if($price) {
    $products = $products->filter(function ($child) use ($price) {
                    return $child->price()->toFloat() <= $price;
                });
}

Let's write like Laravel with when() method same process:

$products = $page->children()
                ->when($size, function ($collection, $size) {
                    return $collection->filterBy('size', $size);
                })
                ->when($color, function ($collection, $color) {
                    return $collection->filterBy('color', $color);
                })
                ->when($price, function ($collection, $price) {
                    return $collection->filter(function ($child) use ($price) {
                        return $child->price()->toFloat() <= $price;
                    });
                });

Lets write with sortBy() method and fallback:

$page->children()
    ->when($sortBy, function ($collection, $sortBy) {
        return $collection->sortBy($sortBy);
    }, function ($collection) {
        return $collection->sortBy('title');
    });

The when method only executes the given Closure when the first parameter is true. If the first parameter is false, the Closure will not be executed.

Don't you think it's very useful and readable?

@afbora afbora changed the title Conditional Clauses on Collection Filtering Conditional Clauses Jul 16, 2019
@distantnative

This comment has been minimized.

Copy link
Contributor

@distantnative distantnative commented Jul 18, 2019

I actually like this quite a bit! :D

@distantnative

This comment has been minimized.

Copy link
Contributor

@distantnative distantnative commented Jul 18, 2019

I can't test any of this at the moment, so this is only a thought concept... but I wanted to write down something as a proof of concept. So this would be a plugin for this:

Kirby::plugin('my/when', [
    'pagesMethods' => [
        'when' => function ($condition, Closure $callback, Closure $fallback = null) {
            if ($condition) {
              return $callback->call($this, $condition);
            }
            
            if ($fallback !== null) {
              return $fallback->call($this, $condition);
            }

            return $this;
        }
    ]
]);

To be used as

$page->children()
    ->when($sortBy, function ($sortBy) {
        return $this->sortBy($sortBy);
    }, function () {
        return $this->sortBy('title');
    });
@afbora

This comment has been minimized.

Copy link
Author

@afbora afbora commented Jul 18, 2019

@distantnative Very impressed 😲
I've already added this method to my new application 😅

In fact, I never thought this would be done with a plugin.
I've done a few tests, now it works fine, perfect!
Would you consider adding this to the Kirby core?

@distantnative

This comment has been minimized.

Copy link
Contributor

@distantnative distantnative commented Jul 18, 2019

I am not sure, we would need to discuss this with @bastianallgeier.
But happy that it works right away. And only shows the power of Kirby's extensibility.

@texnixe

This comment has been minimized.

Copy link

@texnixe texnixe commented Jul 18, 2019

@distantnative The downside of a plugin is that we would have to add this method to every type of collection (pages, files, users ...) because as far as I know there is no way to extend the collection method.

@distantnative

This comment has been minimized.

Copy link
Contributor

@distantnative distantnative commented Jul 18, 2019

@texnixe Indeed. I think there are good reasons to include this in the core, if it's something of broader use for many. That's something I can't really estimate right now.

Sidenote to make the plugin better then:

$methods = [
        'when' => function ($condition, Closure $callback, Closure $fallback = null) {
            if ($condition) {
              return $callback->call($this, $condition);
            }
            
            if ($fallback !== null) {
              return $fallback->call($this, $condition);
            }

            return $this;
        }
    ];

Kirby::plugin('my/when', [
    'pagesMethods' => $methods,
    'filesMethods' => $methods,
    'usersMethods' => $methods
]);
@texnixe

This comment has been minimized.

Copy link

@texnixe texnixe commented Jul 18, 2019

Just out of interest: Is there a reason why collection methods are not extendable?

@distantnative

This comment has been minimized.

Copy link
Contributor

@distantnative distantnative commented Jul 18, 2019

I don't think there is a specific reasoning behind it from Bastian...
More like it started with pages... at some point files and users got added. and no one thought about implementing it more broader for collections (most use cases are probably restricted to one type... but I totally see the case where such more general ones would be great). Shall we open a separate issue for that one?

@texnixe

This comment has been minimized.

Copy link

@texnixe texnixe commented Jul 18, 2019

Shall we open a separate issue for that one?

Would make sense if it doesn't exist yet.

@lukasbestle

This comment has been minimized.

Copy link

@lukasbestle lukasbestle commented Jul 18, 2019

Just out of interest: Is there a reason why collection methods are not extendable?

The method extensions were introduced in Kirby 2 where the different CMS collection types were built on the Kirby\Toolkit\Collection class. Because it was part of the Toolkit, it wasn't possible to extend that from the CMS in the same way as the CMS collections (I mean it would have been possible, but a bit strange and not consistent).

Now that we have a CMS Collection class, that has however become a non-issue.

@distantnative

This comment has been minimized.

Copy link
Contributor

@distantnative distantnative commented Jul 18, 2019

Upvote here: #352

afbora added a commit to getkirby/kirby that referenced this issue Oct 14, 2019
@afbora afbora referenced this issue Oct 14, 2019
4 of 4 tasks complete
bastianallgeier added a commit to getkirby/kirby that referenced this issue Oct 15, 2019
@bastianallgeier

This comment has been minimized.

Copy link
Contributor

@bastianallgeier bastianallgeier commented Oct 15, 2019

@texnixe texnixe referenced this issue Oct 19, 2019
32 of 33 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.