即使存在唯一检查，唯一字符串生成器也会生成重复项

Question

I am making a small class that uses the gfycat wordlists to generate unique strings.

<?php

namespace Jamosaur\Randstring;

class Randstring
{
    private $adjectives;
    private $animals;

    private $min;
    private $max;
    private $case;
    private $maxLength;

    private $string;

    public $combinations = [];

    private $first;
    private $second;

    public $adjective;
    public $animal;
    public $number;

    /**
     * Randstring constructor.
     * @param null $case (ucwords, ucfirst)
     * @param int $maxLength
     * @param int $min
     * @param int $max
     */
    public function __construct($case = null, $maxLength = 100, $min = 1, $max = 99)
    {
        $this->case         = $case;
        $this->maxLength    = $maxLength;
        $this->min          = $min;
        $this->max          = $max;
        $this->adjectives   = explode(PHP_EOL, file_get_contents(__DIR__.'/dictionaries/adjectives.txt'));
        $this->animals      = explode(PHP_EOL, file_get_contents(__DIR__.'/dictionaries/animals.txt'));
    }

    /**
     * @param null $first
     * @param null $second
     */
    public function generateNumbers($first = null, $second = null)
    {
        $this->first    = ($first) ? $first : mt_rand(0, count($this->adjectives) - 1);
        $this->second   = ($second) ? $second : mt_rand(0, count($this->animals) - 1);
        $this->number   = mt_rand($this->min, $this->max);
        if (isset($this->combinations[$this->first.'.'.$this->second.$this->number])) {
            $this->generateNumbers($this->first, $this->second);
        }
        $this->combinations[$this->first.'.'.$this->second.$this->number] = 1;
}

    /**
     * Generate a string.
     */
    public function generateString()
    {
        $this->generateNumbers();
        $this->adjective    = $this->adjectives[$this->first];
        $this->animal       = $this->animals[$this->second];
        switch ($this->case) {
            case 'ucfirst':
                $this->string   = ucfirst($this->adjective.$this->animal.$this->number);
                break;
            case 'ucwords':
            $this->string   = ucfirst($this->adjective).ucfirst($this->animal).ucfirst($this->number);
                break;
            default:
                $this->string   = $this->adjective.$this->animal.$this->number;
                break;
        }
    }

    /**
     * @return mixed
     */
    public function generate()
    {
        $this->generateString();
        if (strlen($this->string) > $this->maxLength) {
            return $this->generate();
        }

        return $this->string;
    }
}

I added a check in to log each combination that has been created in generateNumbers() which should store each combination in an array.

I have set up a small test to generate 10000 unique strings just for performance testing, which is done with this snippet:

$rand = new Jamosaur\Randstring\Randstring(null, 10);
for ($i=0; $i < 10000; $i++) {
    $t[$i] = $rand->generate();
}
echo 'Unique Strings: '.count(array_unique($t)).'<br>';
echo 'Combinations: '.count($rand->combinations).'<br>';

Running this, it is expected that there will be 10000 unique strings.

I ran the test 10 times, and these were the results:

Unique Strings: 9998 Combinations: 527879

Unique Strings: 9999 Combinations: 518899

Unique Strings: 9999 Combinations: 515290

Unique Strings: 9999 Combinations: 516193

Unique Strings: 10000 Combinations: 526652

Unique Strings: 10000 Combinations: 516049

Unique Strings: 10000 Combinations: 523217

Unique Strings: 10000 Combinations: 509729

Unique Strings: 10000 Combinations: 517236

Unique Strings: 10000 Combinations: 512270

Is there a flaw in the logic here somewhere? The test is limited at a 10 character string, but the tests show that there is a minimum of 10000 unique strings.

Answer 1

Several problems within your code:

a) Remembering of used combinations is flawed

$this->combinations[$this->first.'.'.$this->second.$this->number] = 1;

For a given $this->first this will collide for

$this->second = 10, $this->number = 11 (=1011)

$this->second = 101, $this->number = 1 (=1011)

Add a delimitier between $this->second and $this->number

b) There are possibly duplicates within your wordlists I downloaded the files and e.g. the word "green" is a duplicate within adjectives

c) Carefully debug the recursive (self referencing) character of your code.

d) What's the purpose of the arguments within function generateNumbers($first = null, $second = null)?