Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

optional and boolean api inconsistency #265

Closed
wayneashleyberry opened this issue Mar 5, 2014 · 5 comments
Closed

optional and boolean api inconsistency #265

wayneashleyberry opened this issue Mar 5, 2014 · 5 comments

Comments

@wayneashleyberry
Copy link

It feels unintuitive to me that optional and boolean use completely different input values.

Shouldn't they both use percentages?

@fzaninotto
Copy link
Owner

You're right. Unfortunately, changing either one of optionalor boolean will lead to a BC break... which I don't want for now.

Let's keep this open for Faker 2.0.

@wayneashleyberry
Copy link
Author

Excellent! Thanks :)

@EmanueleMinotto
Copy link
Contributor

The mt_rand function accepts only integers, so what about a solution like this for the v1.* versions and a BC for the v2?

$faker->optional(40); // new optional method with percentage
$faker->optional(0.6); // old system

Internally could be:

    public function optional($weight = 0.5, $default = null)
    {
        // case 1
        if (ctype_digit($weight) && mt_rand(1, 100) <= $weight) {
            return $this->generator;
        }

        // case 2
        if ($weight > 0 && $weight < 1 && mt_rand() / mt_getrandmax() <= $weight) {
            return $this->generator;
        }

        return new DefaultGenerator($default);
    }

Use cases

  • $faker->optional(0) conflict but useless imho, in case 1 it always return NULL, in case 2 it never return NULL
  • $faker->optional(0.5) no conflict, case 2 is invoked
  • $faker->optional(1) conflict if we consider both cases, but considering that case 2 with $weight == 1 is always NULL, case 1 is the only with sense
  • $faker->optional(50) no conflict, case 1 is invoked
  • $faker->optional(100) no conflict, case 1 is invoked

I can write a PR if this is ok for you.

@fzaninotto
Copy link
Owner

Great idea! I'd love a PR.

@wayneashleyberry
Copy link
Author

Closed due to inactivity.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants