On Fri, May 26, 2017 at 10:34 AM, Fleshgrinder <p...@fleshgrinder.com> wrote:
> On 5/26/2017 1:08 AM, Marco Pivetta wrote: > > Saw the discussion on github, and I wish that the argument parsing just > > behaved like a *NORMAL* PHP method. > > > > The following is perfectly valid: > > > > $crapTonOfUuids = array_map([UUID::class, 'v4'], range(0, 1000)); > > > > This would raise a lot of warnings if the API didn't behave like it does > in > > userland (warning on too many arguments). > > A point was raised about BC compliance when adding parameters (future > > scope), well here's the news: stop adding arguments to existing > functions, > > make some damn new functions/methods/classes (to whoever still thinks > that > > adding arguments is a valid decision). > > > > Well, I agree on the adding arguments part. I actually complained a lot > about this in the past. > > https://github.com/php/php-src/commit/49aed4fd75e9560444f63593b67fc4 > ed18e233c9#commitcomment-22277780 > > I added them because Kalle and Nikita really want to have them. Changing > the return types to be nullable is a complete no-go for me. I am sure > Larry would agree here with me. > To clarify, I certainly do *not* want the behavior that was implemented here. The correct way (in your specific case) to handle this if by using if (zend_parse_parameters_throw(ZEND_NUM_ARGS(), "") == FAILURE) { return; } or adding a zend_parse_parameters_none_throw() API. Of course this should not throw a warning and of course it should not return NULL, because that would be inconsistent with how the other UUID methods behave. Of course it should not allow silently passing additional arguments, because that would be inconsistent with both how the other UUID methods (with at least one argument) behave and with how PHP in general behaves. Nikita > > The approach I've taken right now would allow one to write: > > $crapTonOfUuids = @array_map([UUID::class, 'v4'], range(0, 1000)); > > As it would emit a warning, but still generate them. Well, unless you > have strict-types mode activated, in that case you would receive the > ArgumentCountError. > > I really don't know what the proper solution for this problem is. I > would just leave it out, as I did initially. Nothing bad can happen from > passing too many arguments; not enough should directly lead to an > ArgumentCountError, that's for sure. > > On 5/26/2017 1:08 AM, Marco Pivetta wrote: > > The UUID type and specification is simple and clear. > > Also, a UUID is a data type with no real behavior. > > The only possible and valid scenario for subclassing would be to add > > semantic meaning because the developer invented a new type of UUID: > that's > > to be excluded, as such an implementation (if relevant and secure) would > > land in core anyway in future PHP releases. > > Subclassing to alter behavior (generation/serialisation, if you want to > > call them "behavior") would be a mistake that could even lead to security > > issues, and it should be avoided. > > This class should be final, so keep it final, IMO. > > > > Marco Pivetta > > > > +1 > > -- > Richard "Fleshgrinder" Fussenegger > >
