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

Allow ArrayCollection#filter to filter by key, value or both #165

Closed
ghost opened this issue Oct 10, 2018 · 5 comments
Closed

Allow ArrayCollection#filter to filter by key, value or both #165

ghost opened this issue Oct 10, 2018 · 5 comments
Assignees
Milestone

Comments

@ghost
Copy link

ghost commented Oct 10, 2018

Scenario
I want be able to filter an array by key or both (key, value)

    /// DOESN"T WORK :(
    (new ArrayCollection(['italian' => 'pizza', 'turkish' => 'kebab']))
        ->filter(function($k, $v) {
            return 'turkish' === $k;
        })
    // WORKSS!1!!  
    array_filter(['italian' => 'pizza', 'turkish' => 'kebab'], function($k, $v) {
            return 'turkish' === $k;
        }, ARRAY_FILTER_USE_BOTH);

Suggested Solution
Since PHP 5.6 we are able to specify a flag on array_filter which allows to filter the element of the array by passing to the closure that filter the array, key, value or both

array array_filter ( array $array [, callable $callback [, int $flag = 0 ]] )

Flag determining what arguments are sent to callback:

ARRAY_FILTER_USE_KEY - pass key as the only argument to callback instead of the value
ARRAY_FILTER_USE_BOTH - pass both value and key as arguments to callback instead of the value

Default is 0 which will pass value as the only argument to callback instead.

Suggested Implementation
At the moment ArrayCollection#filter uses the default value 0 which makes the flag feature impossible to use.

-    public function filter(Closure $p)
+    public function filter(Closure $p, int $flag = 0)
    {
-        return $this->createFrom(array_filter($this->elements, $p));
+        return $this->createFrom(array_filter($this->elements, $p, $flag));
    }

Concerns
I understand that exposing lower level APIs to an higher level of abstraction is not probably a good idea. Yet, the additional feature would be useful overall to the client who uses the method

If you provide some feedback I can submit a PR

@Ocramius
Copy link
Member

@0x13A if this can be implemented without introducing a BC break, I'd say "why not"?

Also, a benchmark is required here, due to this being performance-sensitive API for downstream consumers.

@ghost
Copy link
Author

ghost commented Oct 15, 2018

@Ocramius Thanks for the reply.

Using the default value for $flag = 0 does not introduce a BC Break as it is the default behaviour of array_filter. I will update this issue as soon as I have a PR.

@Ocramius
Copy link
Member

@0x13A I'm suggesting to use the ARRAY_FILTER_USE_BOTH as default, whilst keeping the public API of the method unchanged.

@ghost
Copy link
Author

ghost commented Oct 15, 2018

That makes a lot of sense

@Ocramius
Copy link
Member

Handled in #167

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

No branches or pull requests

1 participant