FakerPHP / Faker

Faker is a PHP library that generates fake data for you
https://fakerphp.github.io
Other
3.49k stars 331 forks source link

Update `randomElements` to return random number of elements when no count is provided #658

Closed calebdw closed 1 year ago

calebdw commented 1 year ago

Hello!

What is the reason for this PR?

This updates randomElements to return a random number of elements when no count is provided. Defaulting to a count of 1 is undesirable because if I only wanted one element I would just use randomElement. This cleans up having to pass fake()->numberBetween(1, count($array)) as the $count when I don't care how many values are returned.

Author's checklist

Summary of changes

Review checklist

bram-pkg commented 1 year ago

Unfortunately changing the default value of a parameter is a breaking change if it changes the output of the function.

calebdw commented 1 year ago

I understand, can we merge to the next major version though? I don't think it really makes sense to have to explicitly pass null to $count for this to behave as it reads.

Or I suppose I could change the default back to 1, and then on the next major release we could update the default value

bram-pkg commented 1 year ago

Indeed, I think allowing null to get a random number of elements sounds fine, but the signature cannot change within 1.x, sorry.

Please be sure to include a test that asserts a count, which should work fine since we are seeding the mt_ functions.

Also please open a PR to the docs repository.

calebdw commented 1 year ago

@bram-pkg done, is there a planned 2.x release date?

bram-pkg commented 1 year ago

Not as of yet. We're struggling to be motivated I think, outside our day jobs :)

DvDty commented 1 year ago

@bram-pkg can the community help with this?

calebdw commented 1 year ago

That's a good idea, it would be helpful if a 'Roadmap to 2.0' issue was created with what needs to be done and then the community could pitch in

pimjansen commented 1 year ago

That's a good idea, it would be helpful if a 'Roadmap to 2.0' issue was created with what needs to be done and then the community could pitch in

Will work on that