1

I'm trying to make a clean array from a string that my users will define. The string can contain non-valid IDs, spaces, etc. I'm checking the elements using a value object in a callback function for array_filter.

$definedIds = "123,1234,1243, 12434 , asdf"; //from users panel

$validIds = array_map(
    'trim',
    array_filter(
        explode(",", $definedIds),
        function ($i) {
            try {
                new Id(trim($i));
                return true;
            } catch (\Exception $e) {
                return false;
            }
        }
    )
);

This works fine, but I'm applying trim twice. Is there a better way to do this or a different PHP function in which I can modify the element before keeping it in the returned array?

NOTE: I also could call array_map in the first parameter of array_filter, but I would be looping through the array twice anyway.

CarlosAS
  • 654
  • 2
  • 10
  • 31
  • 2
    As a small suggestion, don't make the constructor throw, but instead use a statically defined `Id.isValidId(input)` that you can call, so you don't need to incur memory allocation, symbol table modification, and garbage collection. You're not doing anything with the `Id` object you're creating, so if you're using it just to check if an id is valid, make that a dedicated function you can call. – Mike 'Pomax' Kamermans May 01 '18 at 13:50
  • what is your expected output? – A l w a y s S u n n y May 01 '18 at 13:52
  • why don't you use `str_replace` for removing whitespaces before mapping the array ? – mim. May 01 '18 at 13:55
  • 1
    @BeingSunny it's [123,1234,1243,12434] – CarlosAS May 01 '18 at 13:57
  • @mim. that's a good idea for this particular case, but still wondering if there is a function using the callback to fill the returned array – CarlosAS May 01 '18 at 13:59
  • @CarlosAS you may try `array_walk_recursive` then. – mim. May 01 '18 at 14:07
  • Simply call `preg_split('/,\s*/', $string)` to explode on commas with and without trailing spaces. – mickmackusa Aug 11 '22 at 21:32

2 Answers2

1

It depends on whether you care about performance. If you do, don't use map+filter, but use a plain for loop and manipulate your array in place:

$arr = explode(',', $input);

for($i=count($arr)-1; $i>=0; $i--) {
  // make this return trimmed string, or false,
  // and have it trim the input instead of doing
  // that upfront before passing it into the function.
  $v = $arr[$i] = Id.makeValid($arr[$i]);

  // weed out invalid ids
  if ($v === false) {
    array_splice($arr, $i, 1);
  }
}

// at this point, $arr only contains valid, cleaned ids

Of course, if this is inconsequential code, then trimming twice is really not going to make a performance difference, but you can still clean things up:

$arr = explode(',', $input);
$arr = array_filter(
  array_map('Id.isValidId', $arr),
  function ($i) {
    return $i !== false;
  }
);  

In this example we first map using that function, so we get an array of ids and false values, and then we filter that so that everything that's false gets thrown away, rather than first filtering, and then mapping.

(In both cases the code that's responsible for checking validity is in the Id class, and it either returns a cleaned id, or false)

Mike 'Pomax' Kamermans
  • 49,297
  • 16
  • 112
  • 153
0

Actually you can do it by different way but If I were you then I'll do it this way. Here I just used only one trim

<?php
$definedIds = "123,1234,1243, 12434 , asdf"; //from users panel
function my_filter($b){
    if(is_numeric($b)){
        return true;
    }
}
print '<pre>';
$trimmed = array_map('trim',explode(',',$definedIds));
print_r(array_filter($trimmed,my_filter));
print '</pre>';
?>

Program Output:

Array
(
    [0] => 123
    [1] => 1234
    [2] => 1243
    [3] => 12434
)

DEMO: https://eval.in/997812

A l w a y s S u n n y
  • 36,497
  • 8
  • 60
  • 103
  • This is the same code of my question, you just named the callback function and removed the value object checks... – CarlosAS May 01 '18 at 14:46
  • @CarlosAS :P Yes you're ri8, I just unconsciously posted the same kind of logic that you already did. See my edit now I just used only one time **trim** inside array_map() – A l w a y s S u n n y May 01 '18 at 17:40