Refactor drupal_map_assoc()

Yes. Yes it is.

http://coverage.cwgordon.com ;)

It is a limitation in PHP. Only strings and integers are allowed as keys (see the manual page about arrays). In your last example, the key is the string "1.5" (you can see that if you use var_dump() rather than print_r()). Apparently array_combine() casts the keys to strings.

But not with the implementation on my patch, except when passing a function on the second argument (array_map() also strips out the decimal part).

I cannot replicate that. Could you supply a testcase?

+ if (isset($function) && function_exists($function)) {

You might want to replace this with if (is_callable($function)) {. This will allow $function to be a class method specified using the array syntax, e.g. array('MyClass', 'myMethod') or array($object, 'myMethod').

I am running into a bug (?) like this in Drupal 6. I'm coding a site which searches a database of real estate properties, and want people to be able to select the number of bathrooms. "Half-bathrooms" are counted as .5 a bathroom, so a house with two bathrooms and one half-bathroom has 2.5 bathrooms. Unfortunately, I must use PHP 4; "pirets_range()" below is a workaround for PHP 4's inability to use the "$step" parameter of the standard range() function.

$baths = pirets_range(1, 5, .5);
var_dump($baths);
$options = drupal_map_assoc($baths);
var_dump($options);
die();

array(9) {
  [0]=>
  int(1)
  [1]=>
  float(1.5)
  [2]=>
  float(2)
  [3]=>
  float(2.5)
  [4]=>
  float(3)
  [5]=>
  float(3.5)
  [6]=>
  float(4)
  [7]=>
  float(4.5)
  [8]=>
  float(5)
}
array(5) {
  [1]=>
  float(1.5)
  [2]=>
  float(2.5)
  [3]=>
  float(3.5)
  [4]=>
  float(4.5)
  [5]=>
  float(5)
}

I thought about hacking drupal_map_assoc() in various ways -- strvaling all keys, all the time, or maybe just strvaling the keys if it looks like any of the parameters are not ints -- though that would require pre-looping through the array for possibly the entire length of it… Not increasing the execution time by up to 2x would be a good idea, I think. Eventually I decided to try converting the values to strings before passing them to drupal_map_assoc(). But that exposes another curious behavior!

$baths = pirets_range(1, 5, .5);
var_dump($baths);
$options = drupal_map_assoc(array_map('strval', $baths));
var_dump($options);
die();

array(9) {
  [0]=>
  int(1)
  [1]=>
  float(1.5)
  [2]=>
  float(2)
  [3]=>
  float(2.5)
  [4]=>
  float(3)
  [5]=>
  float(3.5)
  [6]=>
  float(4)
  [7]=>
  float(4.5)
  [8]=>
  float(5)
}
array(9) {
  [1]=>
  string(1) "1"
  ["1.5"]=>
  string(3) "1.5"
  [2]=>
  string(1) "2"
  ["2.5"]=>
  string(3) "2.5"
  [3]=>
  string(1) "3"
  ["3.5"]=>
  string(3) "3.5"
  [4]=>
  string(1) "4"
  ["4.5"]=>
  string(3) "4.5"
  [5]=>
  string(1) "5"
}

So I've got the right number of elements this time, but the keys are cycling back and forth between ints and strings! Aah, the joy of duck-typed languages.

In my case this is acceptable behavior, though, and I presume it will be in most others' cases as well.

So I've got the right number of elements this time, but the keys are cycling back and forth between ints and strings!

This is also a (somewhat surprising) PHP limitation/feature. If a key is a string containing an integer it is converted to that integer, i.e. string(1) "3" is converted to int(3).

Though this behaviour may appear a bit weird, I have managed to program PHP for years without even noticing it, so I guess it wont matter to most people. At least there is nothing that Drupal can do about it.

Looks reasonable.

Incorporated c279364's feedback from #4.

#367711: Shorten drupal_map_assoc() approached almost the same, and contains some additional concerns:

- Handling of empty arrays.

- Usage of array_walk() instead of array_map().

Sigh. I rewrote this yet another time and just found this patch. Hope someone can track down these test exceptions.

Looking at the new drupal_map_assoc, I'm thinking that it can just go away. Just about every caller can just do array_combine($array, $array) and get on with life.

Why is it still green then?

Ah, indeed it is. RTBC then.

err? :-D

Committed to CVS HEAD.