This generator does not guard against duplicate numbers being issued.

Edit: Good idea though, and very welcome contribution if you can take it a step further :)

Yes, we should perform a check to avoid duplicate numbers. What do you think @mykehsd ?

@pjedrzejewski I don't really like the for() in this solution but we wouldn't want to indefinitely look for order numbers. Other ideas?

Try a do...while(). And for now I would not worry about the indefinite part, should not be a real-world issue, and if it becomes one, we can introduce a limit parameter later.

@Richtermeister That wouldn't be testable in phpspec afaik
What's the advantage over for() ?

Why would it be not testable? The advantage is that you don't just loop for an arbitrary number of times, as you do, but you loop until a condition is fulfilled.

The failure event wouldn't be testable since it will never exit that loop.

You mean because you can't tell phpspec mocks to return false once and true the second time? You actually can, by providing a custom handling function.

That would be a good addition but I'm talking about the issue where isUnused() never returns true

Why would it never return true? In reality we're just guarding against the occasional collision, unlikely to be more than one in a row, and even if there were to be many more, you should always get to a point where an unused number is found. Otherwise the problem is more with the hash generation strategy and the entire generator should be reworked.

In respect to

        $numberRepository->isUnused(Argument::any())->willReturn(false);

it would never return (infinite loop). In reality, the hashing strategy will very unlikely collide with an existing number but I think throwing an exception is better then an infinite loop

In general the spec would be sufficient to test that the method is just being called. You're not testing the repository, after all. If you really wanted to test against 2 executions of the loop, you can use something like this:

$counter = 0;
$numberRepository->isUnused(Argument::any())->willReturn(function() use (&$counter){
    $counter++;
    return $counter > 1;
});

@Richtermeister ok, do...while added. Thanks for your feedback!

I don't know why Travis shows errored, but it shows passed