Skip to content

Conversation

@timacdonald
Copy link
Member

@timacdonald timacdonald commented Jun 23, 2022

tl;dr; ☹️☹️🤑☹️☹️☹️🤑

This PR introduces a new Lottery class. In isolation, this is not super useful, but when used with other features within Laravel and your own application, it can make for a nice experience.

Lotteries can be used in an assortment of ways, for example to do random sampling of an event or assign users to an A/B test group, etc.

Lottery::odds(1, 20)
    ->winner(fn () => $user->group = 'treatment')
    ->loser(fn () => $user->group = 'control')
    ->choose();

It is also useful with built in framework features...

As the Lottery class is callable, it may be passed directly in as a handler to some of Laravel's features. The following is an example of its use with the new DB::whenQueryingForLongerThan functionality.

// Randomly sample so we don't flood our error handler...

DB::whenQueryingForLongerThan(
    Interval::seconds(2),
    Lottery::odds(1, 100)->winner(fn () => report('DB queries exceeded 2 seconds'))
);

It could also be utilised with the existing n+1 reporting. This is especially useful because unlike the above function, the n+1 handler function may be called thousands of times per request...

// Randomly sample so we don't flood our error handler...

Model::handleLazyLoadingViolationUsing(Lottery::odds(1, 5000)->winner(function ($model, $relation) {
    report(new LazyLoadingViolationException($model, $relation));
}));

You will notice in the above examples that we did not call ->choose(). This is because, an previously mentioned, the Lottery is callable. Additionally you will notice that the closures passed to the lottery receive the arguments $model and $relation.

I wrote a blog post introducing this idea of randomly sampling lazy loading violations via lottery after we implemented this at my $previousGig

Full API examples

/*
 * Via ->choose()
 */

Lottery::odds(1, 10)->choose();

// returns `true` for "win"
// returns `false` for "loss"

Lottery::odds(1, 10)->winner(fn () => 'winner')->choose();

// returns `"winner"` for "win"
// returns `false` for "loss"

Lottery::odds(1, 10)->loser(fn () => 'loser')->choose();

// returns `true` for "win"
// returns `"loser"` for "loss"

Lottery::odds(1, 10)
    ->winner(fn () => 'winner')
    ->loser(fn () => 'loser')
    ->choose();

// returns `"winner"` for "win"
// returns `"loser"` for "loss"

Lottery::odds(1, 5)
    ->winner(fn () => 'winner')
    ->loser(fn () => 'loser')
    ->choose(10);

// example result...
// [
//     "loser",
//     "loser",
//     "loser",
//     "winner",
//     "loser",
//     "winner",
//     "loser",
//     "winner",
//     "loser",
//     "loser",
// ]

/*
 * Via ->__invoke()
 */

// Same as above, however it works different to "choose" as the 
// arguments passed in are passed through to the winner / loser closure.
// This is mostly useful when passing to a function that accepts a callable...

$lottery = Lottery::odds(1, 10)
    ->winner(fn ($arg1, $arg2) => "{$arg1} {$arg2} baz")
    ->loser(fn ($arg1, $arg2) => "{$arg1} {$arg2} qux");

$lottery('foo', 'bar');
// returns `"foo bar baz"` for "win"
// returns `"foo bar qux"` for "loss"

/*
 * Finally...
 */
   
Lottery::setResultFactory(function (): bool {
    // Set your own algorithm for determining results if the built in one doesn't meet your needs.
}); 

This new class could also be utilised in the framework where we perform lotteries, i.e. in the session and database lock garbage collection. I've not included that in this PR as I don't think we need to do it just for the sake of it. This is, after all, primarily a new user-facing feature.

Lottery::odds(...$this->lottery)->winner(
    fn () => $this->connection->table($this->table)->where('expiration', '<=', time())->delete()
)();

I've also added some testing helpers similar to those found in other Laravel support classes.

Lottery::alwaysWin();

// Lottery will now always win.

// -------

Lottery::alwaysWin(function () {
    // Lottery will always win when called within this closure.
});

// -------

Lottery::alwaysLose();

// Lottery will now always lose.

// -------

Lottery::alwaysLose(function () {
    // Lottery will always lose when called within this closure.
});

// -------

Lottery::forceResultWithSequence([true, false]);

// lottery will win then lose, and finally return to random results.

// -------

Lottery::forceResultWithSequence([true, false], function (): false {
    // what to do when missing items.
});

// lottery will win then lose, and finally call the closure.

// -------

Lottery::determineResultNormally();

// lottery results return to the "normal" method for determining results.

@fourstacks
Copy link

fourstacks commented Jun 23, 2022

Absolutely love this idea. I can think of several apps where seeding a set of data that needs to skew a certain way was very necessary and the pain points involved in getting that set up to work were pretty real! Think the API for this is spot on - hope it gets merged

@lukeraymonddowning
Copy link
Contributor

Love it!

I wonder if check would be better than choose? Like I'm checking if I won the lottery?

$winner = Lottery::odds(1, 10)->check();

$sample = Lottery::odds(1, 100)->check(10);

@lukeraymonddowning
Copy link
Contributor

Also, could this PR be extended to use this implementation for the existing session lottery?

@timacdonald
Copy link
Member Author

Moving to draft, as I just realised I need to add some testing helpers so you can control the outcome in your tests! Will follow up tomorrow with them.

Thanks for the feedback everyone. Keep it coming. Will respond tomorrow 💖

@timacdonald timacdonald marked this pull request as draft June 23, 2022 10:43
@mattkingshott
Copy link
Contributor

Fantastic!! 🎉

@timacdonald
Copy link
Member Author

timacdonald commented Jun 24, 2022

Also, could this PR be extended to use this implementation for the existing session lottery?

It absolutely could be, but I didn't want to make that change just for the sake of it. If Taylor is keen on making that change, I'd be happy to update the PR to migrate the existing lotteries (session and DB locks) over to this functionality.

Love it!

I wonder if check would be better than choose? Like I'm checking if I won the lottery?

$winner = Lottery::odds(1, 10)->check();

$sample = Lottery::odds(1, 100)->check(10);

I don't mind this either! I was thinking "choosing" a winner, but I also like "check".

Would love thoughts on the naming for others either way!

@DarkGhostHunter
Copy link
Contributor

This is excellent.

@markjaquith
Copy link
Contributor

The requirement to pass in the numerator and denominator might be a little restrictive. I could think of situations where I have a float between 0 and 1 that is generated by something else and I'd like that to determine the lottery odds.

If you accepted the odds in this form, anyone could do Lottery::odds($probability) or Lottery:odds(1/7) to construct the odds on the fly. Otherwise, you'd have to do something weird like: Lottery::odds(round($probability * 10000), 10000)

@bert-w
Copy link
Contributor

bert-w commented Jun 24, 2022

Firstly, let me state that I voted down because the contents of this class can too easily be distilled to:

if(rand(0,99) >= 75) { 
 // win
} else {
 // lose
}

Therefore I consider it bloat.

If however it would become a part of the framework, it would be nice if you could simply construct using new Lottery(0.9) to indicate a 90% chance of "winning". The 0-1 range is somewhat of a defacto standard within the probability field.

@JayBizzle
Copy link
Contributor

I would agree with some of the other comments about allowing other ways of setting the odds, especially a float between 0 and 1.

@deleugpn
Copy link
Contributor

@bert-w could you write a phpunit test testing both the if and else condition on your snippet as easily and elegant as with the proposed Lottery class? Lottery::alwaysLose() is lit 🔥

@timacdonald
Copy link
Member Author

Appreciate the ongoing feedback folks. Will push an update to support a single float value as probability early next week.

@bert-w
Copy link
Contributor

bert-w commented Jun 25, 2022

@bert-w could you write a phpunit test testing both the if and else condition on your snippet as easily and elegant as with the proposed Lottery class? Lottery::alwaysLose() is lit 🔥

You have a point there. There is an odd solution available though:

// Set seed for random number generator:
srand(1234);
var_dump(rand(0,99));

The result of rand() is now static. However you need to run the random function once to know what the output is, so that's not pretty. Also you cannot use this with random_int() (which is crypto-secure) so that's a limitation.

@nedwors
Copy link
Contributor

nedwors commented Jun 27, 2022

Noice! How about Lottery::drawUsing(...) instead of setResultFactory(...)?

@timacdonald
Copy link
Member Author

Hey folks, I'm gonna temporarily close this one, but it will re-open it shortly.

I'm gonna try and get a few additional features in place which will make this PR even more meaningful to the framework and community.

Stay tuned...

@timacdonald
Copy link
Member Author

So I force pushed and I can't re-open this particular pull request, so I have created a new one: #44894

@timacdonald timacdonald mentioned this pull request Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.