Anonymous function content in PHP not working

Issue

create_function takes two strings, the arguments and body of the function to create. Internally, it uses eval to create something callable, which is generally frowned upon, as it increases the attack surface area.

From your description, the get_usort_function function returns a string; as you noted, if called like:

get_usort_function('field2 ASC, field5 ASC')

it would return something like:

$field2=compare_ints($a['field2'], $b['field2']); if($field2==0){return compare_ints($a['field5'], $b['field5']);}else{return $field2;}

You'd noted that hardcoding the string in the callable passed to usort works, which I'm imagining is something like:

usort($myarray, function($a,$b) { $field2=compare_ints($a['field2'], $b['field2']); if($field2==0){return compare_ints($a['field5'], $b['field5']);}else{return $field2;} } );

but a more accurate hardcoding, given the above description of how get_usort_function works, would be:

usort($myarray, function($a,$b) { "$field2=compare_ints($a['field2'], $b['field2']); if($field2==0){return compare_ints($a['field5'], $b['field5']);}else{return $field2;}" } );

When written out like this, it's clear that changing the usort invocation from create_function to using a callable as you've indicated above won't return anything (so usort will leave all the elements in their existing order). This may be the part that wasn't clear in your understanding of how it was working.

Solution

You could do something like the following (which may be similar to a simplified version of the internal logic of the existing get_usort_function):

<?php

function print_people($people) {
  foreach($people as ['id' => $id, 'name' => $name]) {
    print("{$id}: {$name}\n");
  }
  print("\n");
}

function get_usort_callable(...$fields) {
  return function ($a, $b) use ($fields) {
    foreach ($fields as $field) {
      $result = $a[$field] <=> $b[$field];
      if ($result != 0) { return $result; }
    }
    return 0;
  };
}

$a1 = ['id' => 9, 'name' => 'Andy'];
$a2 = ['id' => 6, 'name' => 'Carol'];
$a3 = ['id' => 5, 'name' => 'Bob'];
$a = [$a1, $a2, $a3];

print_people($a);

usort($a, get_usort_callable('id', 'name'));

print_people($a);

usort($a, get_usort_callable('name', 'id'));

print_people($a);

which gives the following output:

9: Andy
6: Carol
5: Bob

5: Bob
6: Carol
9: Andy

9: Andy
5: Bob
6: Carol

The main takeaway here is using the use keyword on the anonymous function returned by get_usort_callable to make $fields available for use. If you wanted to match the functionality of the existing get_usort_function you could rewrite it to take a string, and split that up.

Minimal work

Given that you are going to have to move away from create_function to use PHP 8, the least amount of work you can do will be to rewrite get_usort_function to return a callable (similar to above) and replace invocations like:

usort($myarray, create_function('$a,$b', get_usort_function(...)));

with:

usort($myarray, get_usort_function(...));

Given that you have access to the internal logic of get_usort_function it should not be too difficult, and thankfully that is only in one spot. The refactoring of the call-sites is pretty mechanical too, almost a find and replace with any IDE.

Going forward (and depending on your preference), you may want to replace the SQL ORDER BY style string with a structured array, eg:

'field2 ASC, field5 ASC'

becomes:

[
  [
    'field' => 'field2',
    'direction' => 'asc'
  ],
  [
    'field' => 'field5',
    'direction' => 'asc'
  ]
]

to avoid possible issues with whitespacing etc that require additional handling in get_usort_function.

create_function vs anonymous functions

When moving from uses of create_function to using anonymous functions, you no longer need to have the arguments and body of the function passed as strings. For example, the following two callables are equivalent:

create_function('$a,$b', 'return $a["id"] <=> $b["id"];')

function($a, $b) { return $a["id"] <=> $b["id"]; }

and could be used interchangeably.

Adding in one level of indirection (as you have in your question), through use of the following functions:

function sort_by_function_body($field) {
  return "return \$a[\"{$field}\"] <=> \$b[\"{$field}\"];";
}

function sort_by_callable($field) {
  return function($a, $b) use ($field) { return $a[$field] <=> $b[$field]; };
}

these two are also equivalent:

create_function('$a,$b', sort_by_function_body('id'))

sort_by_callable('id')

The main takeaway from this is that the sort_by_callable function itself returns a specialised anonymous function, which will sort on the field passed in, as opposed to a string containing the code to perform the same logic.

Thanks!

I've taken what you've said and reworked things to get a working solution that I need...

$a1 = array( 'id' => 7, 'name' => 'Andy', 'dob' => '01-02-2020');
$a2 = array( 'id' => 7, 'name' => 'Bob', 'dob' => '01-02-2022');
$a3 = array( 'id' => 9, 'name' => 'Guy', 'dob' => '01-02-2021');
$a4 = array( 'id' => 7, 'name' => 'Mick', 'dob' => '01-02-2023');
$a5 = array( 'id' => 3, 'name' => 'Daz', 'dob' => '01-02-2019');
$a6 = array( 'id' => 7, 'name' => 'Daz', 'dob' => '01-02-2021');

$a = array($a1, $a2, $a3, $a4, $a5, $a6);

function get_usort_function_callable($order_bys, $mode = 1)
{
    return function ($a, $b) use ($order_bys, $mode)
    {
        // We get the options in reverse order, so we can properly nest them.
        $order_by_options_r = array_reverse(explode(",", $order_bys));
        #print_r($order_by_options_r);
        #print "MODE = $mode";

        $first_element = TRUE;
        
        foreach ($order_by_options_r as $order_by)
        {
            if (( isset($retval) ) AND ( strlen($retval) > 0 ))
            {
                $inner_val = $retval;
            }

            $order_by = trim($order_by);
            $indexOfSpace = strpos($order_by, " ");

            if( $indexOfSpace !== FALSE )
            {
                $column = trim(substr($order_by,0,$indexOfSpace));
                $sortorder = trim(substr($order_by,$indexOfSpace));

                if ( strcasecmp($sortorder,"DESC") == 0 )
                {
                    switch ( $mode )
                    {
                        case 1:
                        case 2:
                            $comp = '$b[' . "'" . $column . "'" . '] <=> $a[' . "'" . $column . "'" . ']';
                        break;
                        case 3:
                            $comp = 'strtotime($b[' . "'" . $column . "'" . ']) <=> strtotime($a[' . "'" . $column . "'" . '])';
                        break;
                        case 4:
                            $comp = 'strnatcmp($b[' . "'" . $column . "'" . '], $a[' . "'" . $column . "'" . '])';
                        break;
                    }
                } else {
                    switch ( $mode )
                    {
                        case 1:
                        case 2:
                            $comp = '$a[' . "'" . $column . "'" . '] <=> $b[' . "'" . $column . "'" . ']';
                        break;
                        case 3:
                            $comp = 'strtotime($a[' . "'" . $column . "'" . ']) <=> strtotime($b[' . "'" . $column . "'" . '])';
                        break;
                        case 4:
                            $comp = 'strnatcmp($a[' . "'" . $column . "'" . '], $b[' . "'" . $column . "'" . '])';
                        break;
                    }
                }

                if($first_element)
                {
                    $retval = 'return ' . $comp . ';';
                    $first_element = FALSE;
                } else {
                    $retval = '$' . $column . '=' . $comp . '; if ( $' . $column . ' == 0 ) { ' . $inner_val . ' } else { return $' . $column . '; }';
                }
            }
        }

        #print "RET = $retval\n";

        return eval($retval);
    };
}

usort($a, get_usort_function_callable("dob ASC, id DESC, name DESC") );

print_r($a);

This works pretty much exactly as I need it now, with the spaceship operator allowing me to get rid of the extra "helper" functions (i.e. compare_dates, compare_ints) and the final eval (of the string that was produced) meaning that the function is returned rather than a string of the function.

Whether I should really be using eval() I'm not sure - prob not, but given that the content of the function is dynamic (i.e. due to the fields being sorted by being dynamic) then I think it's maybe justified.

Now I just need to rework the code slightly, so that all of these calls...

usort($wishlist_details, create_function('$a,$b', get_usort_function('wishlistcountryname ASC, wishlistname ASC')));

...become calls like this...

usort($_wishlist_details, get_usort_function_callable('wishlistcountryname ASC, wishlistname ASC') );

Better get my regex skills sorted for a global search/replace rather than manually editing each file!

Thanks for your help :-)