Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118118
MIME-Version: 1.0
References: <CAMs36HcGzvwwTTU+nUO+U_M6o4qpOAHES7RbHEJWm6iot+-9=Q@mail.gmail.com>
 <CABTDVhA8K3LLZU9--a7vXxUE7wmVJEy8yeXZ9JjXfKVDBTOsHA@mail.gmail.com>
 <CAC5V6gja7ZYX_aB+jrGu4ANsdfAVbZG9DYWYxNPSGwsu=JViGQ@mail.gmail.com> <CAMs36HdH2n6asM7+3J+A0cxTQhNfxWFEgjx7oasy3LD85k4Cjw@mail.gmail.com>
In-Reply-To: <CAMs36HdH2n6asM7+3J+A0cxTQhNfxWFEgjx7oasy3LD85k4Cjw@mail.gmail.com>
Date: Wed, 29 Jun 2022 10:40:11 +0200
Message-ID: <CAC5V6giAtyDtEybCJu=5keb=_xUh0pY05KsnZBu0+gciOYzw2g@mail.gmail.com>
To: Go Kudo <g-kudo@colopl.co.jp>
Cc: internals@lists.php.net
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
From: guilliam.xavier@gmail.com (Guilliam Xavier)

>> > https://wiki.php.net/rfc/random_extension_improvement
>>
>> I just realized a little thing: in the array_rand() example, for
>> $beforeSingle, it would probably be "more realistic" to omit `, 1`
>> (which is already the default for $num).
>>
>> Note: for `Randomizer::pickArrayKeys(array $array, int $num): array`,
>> it makes sense that $num does *not* have a default value (1 would be
>> "weird" because the method always returns a *list of keys*, and
>> count($array) [via null] would be "useless" because keys are returned
>> *in their original order* [so it would make the method equivalent to
>> array_keys($array) by default]),
>> and that's probably a good thing (it forces to update the call by
>> adding an explicit `, 1` argument and reminds to add a `[0]` or
>> similar on the returned value).
>>
>> An alternative design would be `Randomizer::pickArrayKey(array
>> $array): int|string`, but migrating existing uses with $num != 1 would
>> be harder, so probably not better.
>
> This is certainly a complicated issue.
>
> I proposed the signature `Randomizer::arrayPickKeys(array $array, int $num): array` because it can be solved with the current PHP sugar syntax and the default value of $num is 1 despite the name "arrayPickKeys".
>
> However, this is a bit tricky and may not be user-friendly for the average user.
>
> So, how about adding two methods, `Randomizer::arrayPickKey(array $array): int|string` and `Randomizer::arrayPickKeys(array $array, int $num): array`?
>
> This may seem redundant, but it may avoid user confusion.

Sorry if I wasn't clear: I just suggested to make this little change
in the example:

```diff
-$beforeSingle = array_rand(['foo' => 'foo', 'bar' => 'bar', 'baz' =>
'baz'], 1); // (string) foo
+$beforeSingle = array_rand(['foo' => 'foo', 'bar' => 'bar', 'baz' =>
'baz']); // (string) foo
```

to make it more "realistic".

As concerns the rest (about pickArrayKeys): sorry for the digression,
I was just "thinking out loud", I *don't* want any change there
(first, it makes sense that pickArrayKeys has `int $num` *without* a
default value [even if array_rand has 1]; second, "pickArrayKey" [if
really wanted] is trivial to implement in userland as a wrapper around
pickArrayKeys [but the opposite would not be so], and I don't think
that adding *both* methods to Randomizer is desirable either [better
keep it simple/minimal]).

Regards,

-- 
Guilliam Xavier