4
\$\begingroup\$

I needed a PHP function that would filter values from one array into a new one, while also removing those values from the original array. After looking through the docs, I didn't see any such function, so I wrote my own. While I don't expect it to run with any comparable efficiency as a native function, I am curious if there are any optimization steps I could be missing.

This code is written for PHP 8.1.X

/**
 * Extract values from an array, and return the results
 * 
 * @param array             $array      The array to filter
 * @param callable|string   $callback   Function to filter the array with
 * 
 * @return array A new array containing the filtered elements
 */
function array_excise(array &$array, callable|string $callback): array {
    $results = [];
    $arr_len = count($array);
    for($i = 0; $i < $arr_len; $i++) {
        if(is_callable($callback))
        {
            if($callback($array[$i]))
            {
                $results[] = $array[$i];
                unset($array[$i]);
            }
        }
        else if(is_string($callback))
        {
            if(call_user_func($callback, $array[$i]))
            {
                $results[] = $array[$i];
                unset($array[$i]);
            }
        }
    }

    $array = array_values($array);

    return $results;
}

Usage Example:

$people = [
    [
        "name" => "Billy Jean",
        "filter" => false,
    ],
    [
        "name" => "Ronald Reagan",
        "filter" => true,
    ],
    [
        "name" => "Bill Clinton",
        "filter" => true,
    ],
    [
        "name" => "Michael Jackson",
        "filter" => false,
    ],
    [
        "name" => "Johnny Cash",
        "filter" => false,
    ],
];

$presidents = array_excise($people, function($p){ return $p["filter"]; });

print_r($people);

print_r($presidents);

/* output
Array
(
    [0] => Array
        (
            [name] => Billy Jean
            [filter] =>
        )

    [1] => Array
        (
            [name] => Michael Jackson
            [filter] =>
        )

    [2] => Array
        (
            [name] => Johnny Cash
            [filter] =>
        )

)

Array
(
    [0] => Array
        (
            [name] => Ronald Reagan
            [filter] => 1
        )

    [1] => Array
        (
            [name] => Bill Clinton
            [filter] => 1
        )

)
*/
\$\endgroup\$
0

3 Answers 3

Reset to default
5
\$\begingroup\$
  • It might be worth mentioning in your docblock declaration that the input array is required to be an indexed array in your code.
  • Your code returns an indexed array and unconditionally re-indexes the mutated input array. There are scenarios where indexed arrays may damage valuable associative keys. I'll recommend using a foreach() to iterate the dataset and avoid forcing indexed keys. If the code that calls this function needs either of the arrays to be indexed, array_values() is not an expensive or ugly call.
  • You should not write the is_callable() check inside the loop -- it will pass or fail but will not change. Make the check before looping and return early if the parameter is not callable.
  • Both styles of calling the dynamic callback will work regardless of if you declared a string name or an anonymous function, to keep your script lean, just either choose call_user_func() or the variably named function for easier reading.
  • As a matter of PSR-12 compliance, please do not write else if in PHP -- it is a single word.

Suggested code: (Demo) (or Demo)

function array_excise(array &$array, callable|string $callback): array {
    $result = [];

    if (!is_callable($callback)) {
        return $result;
    }

    foreach ($array as $key => $value) {
        if ($callback($value)) {
            $result[$key] = $value;
            unset($array[$key]);
        }
    }
    return $result;
}

Granted your function is called "excise" -- which infers "cutting away" -- I can appreciate that your function mutates the input array. However, I do like what @lukas.j mentions about not mutating the input and instead returning all arrays.

If you wanted to move toward that approach, you could set up a grouping function that creates first level keys from the passed in callback parameter. If your callback returns a boolean result, then expect to handle 0 and 1 group keys. Something like this: (Demo)

function array_group(array $array, callable|string $callback): array {
    $groups = [];
    if (!is_callable($callback)) {
        return $groups;
    }
    foreach ($array as $key => $value) {
        $groups[$callback($value)][$key] = $value;
    }
    return $groups;
}

var_export(
    array_group(
        $people,
        fn($p) => substr($p["name"], 0, 4)
    )
);
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for these optimizations. I do prefer the "excise" methodology, though maybe I'll change the name to something like array_slice_by_filter. That may better convey that the input is expected to be modified. Nonetheless the steps you've outlined are just what I was looking for. \$\endgroup\$ Commented Jul 19, 2022 at 12:36
  • \$\begingroup\$ @GeoffreyH. Excise is a good name. Slice sort of implies taking a contiguous stretch, but this is not necessary for this function. \$\endgroup\$ Commented Jul 19, 2022 at 18:35
4
\$\begingroup\$

Generally it is not considered too good a practice to have a function which mutates arguments and at the same time returns values. Side note: Your documentation block does not mention the mutation of the parameter $array.

It might make more sense to build two arrays in the function and return them together, and use array destructuring to assign the two arrays to two variables:

function array_excise(array $array, callable|string $callback): array {
  if (!is_callable($callback)) {
    return [];
  }
  $valids = [];
  $invalids = [];
  foreach ($array as $key => $value) {
    if ($callback($value)) {
      $valids[$key] = $value;
    } else {
      $invalids[$key] = $value;
    }
  }
  return [ $valids, $invalids ];
}

[ $presidents, $people ] = array_excise($people, fn($p) => $p["filter"]);

But what really would be the solution is to use array_filter:

$presidents = array_filter($people, fn($item) => $item['filter']);
$nonPresidents = array_filter($people, fn($item) => !$item['filter']);
\$\endgroup\$
4
  • \$\begingroup\$ The name I've seen most other languages and libraries use for such a function is partition. Ruby, lodash, kotlin, haskell. In PHP I'd suggest array_partition \$\endgroup\$ Commented Aug 2, 2022 at 11:07
  • 1
    \$\begingroup\$ @Boude That is a good comment, which would maybe make more sense as comment below the Question itself, since the function name and signature come from there. \$\endgroup\$ Commented Aug 2, 2022 at 16:14
  • 1
    \$\begingroup\$ I made it specifically on your answer, because you changed the signature by returning a pair of arrays instead of modifying the argument, which I do agree with is the better choice. That specific signature in other languages is known as partition. I haven't come across a function similar to the question's in other languages, mostly because those would prefer (as you did) a partition instead \$\endgroup\$ Commented Aug 10, 2022 at 10:02
  • 1
    \$\begingroup\$ @Boude: Of course, makes sense! \$\endgroup\$ Commented Aug 10, 2022 at 17:04
1
\$\begingroup\$

Overall the code is readable. The other answers have offered great analysis of the code. It seems like a combination of array_filter() and array_diff() in a single function. I agree with mickmackusa's answer that foreach could be used instead of for. foreach could be used to iterate over the values in various formats - e.g.

foreach ($array as $i => $value) {

Though if only the keys are needed then array_keys() can be used:

foreach (array_keys($array) as $i) {

As this SO answer explains

...I understand wanting to make things more efficient, but I think you're wasting your time for a micro-optimization. Remember, Premature Optimization Is The Root Of All Evil...

The answer by mickmackusa mentions this:

As a matter of PSR-12 compliance, please do not write else if in PHP -- it is a single word.

Many PHP programmers follow standards outlined in PHP Standards Recommendations like PSR-1 and PSR-12 in order to write idiomatic code and follow common conventions.

In PSR-12 section 4.4 it states:

4.4 Methods and Functions

Visibility MUST be declared on all methods.

Method names MUST NOT be prefixed with a single underscore to indicate protected or private visibility. That is, an underscore prefix explicitly has no meaning.

Method and function names MUST NOT be declared with space after the method name. The opening brace MUST go on its own line, and the closing brace MUST go on the next line following the body. There MUST NOT be a space after the opening parenthesis, and there MUST NOT be a space before the closing parenthesis.

So the declaration for array_excise can be updated so the opening brace is on its own line:

function array_excise(array &$array, callable|string $callback): array 
{

Conversely in PSR-12 section 5 it states:

5. Control Structures

The general style rules for control structures are as follows:

  • There MUST be one space after the control structure keyword
  • There MUST NOT be a space after the opening parenthesis
  • There MUST NOT be a space before the closing parenthesis
  • There MUST be one space between the closing parenthesis and the opening brace
  • The structure body MUST be indented once

So conditionals like this:

if(is_callable($callback))
{

can have the new line removed to follow the convention.

if (is_callable($callback)) {
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.