On Wed, Apr 3, 2013 at 4:52 AM, Sara Golemon <poll...@php.net> wrote:
> https://wiki.php.net/rfc/php-array-api > I like the idea behind this, though right now that API seems a bit like a combinatorial explosion of functions and I think it would be better if one could separate different parts of it. So some feedback: 1. The list of type modifiers seems excessive and the single-character shorthands are confusing (especially as this particular set doesn't seem to be employed anywhere else in our APIs). So some particular suggestions: a) The "c" modifier seems like an unnecessary microoptimization. Compilers should be able to optimize strlen() calls on constant strings away and even if they didn't, it wouldn't be much of a big deal. Also using the "c"-variants on a non-literal would not throw an error and just use an invalid length instead. b) Imho the "l_safe" case does not need its own modifier. Typically strings in PHP are always zero-terminated (in debug mode you'll actually get warnings if they are not). The cases where they aren't zero-terminated are rare and in these cases one can just write those two extra lines of code to do the right thing. c) I think it would be a lot more intuitive if we used the terminology of the normal array APIs instead of the shorthands: php_array_fetch => php_array_fetch_assoc php_array_fetchl => php_array_fetch_assoc_ex php_array_fetchn => php_array_fetch_index php_array_fetchz => php_array_fetch_zval 2. The php_array_fetch*_* APIs currently combined two things: a) Fetching from the array and b) Casting it to some type. I think both should be separate. Not only to avoid the combinatorial explosion of different modifier+type combinations, but also because those casting methods are also applicable to other contexts, not just arrays casts. I asked some time ago to add functions that can directly get a long/double from a zval (though didn't pursue this further). Your APIs add something like that, but tightly coupled to array fetches. There would be more use for it if it were separate :) Thanks, Nikita
