GamePicker now picks the same game each day for each set of players

[Serneum]

TODO: Write tests

Closes #23

[daveshah]

So, this and the call to Enum.take_random are non-deterministic...
That said, if you want to test this and allow for this to be deterministic, maybe a "game randomization" function would be in order - something that you could inject.

There's a number of ways to inject it, but what this could look like:

def pick_game(games_list, num_players, randomizer \\ GamePicker.Randomizer) do 
...
|> randomizer.randomize()
end

You could then test the injected piece independently if you want.
Just a thought.

[mbramson]

Does Enum.take_random not use the value seeded to :rand.seed/2?

[Serneum]

It does

[Serneum]

Looking at something like this:

defmodule Randomizer do
  def new do
    :rand.seed(:exsplus, DateTime.utc_now |> DateTime.to_date |> Date.to_erl)
  end

  def new(seed) do
    :rand.seed(:exsplus, seed)
  end
end

Is that how the randomizer would work with injection?

EDIT: this doesn't have a randomize and I'm not sure how that would work

[rclark72]

@daveshah I'd like to respectively disagree that this is non-deterministic. Given the same seed and the same pseudorandom algorithm, you should always get the same output. The seed is what gives the RNG its non-determinism (which we're taking away)

Try it yourself:

:rand.seed(:exsplus, {10, 17, 2016})
:rand.uniform(1000)

Should always return 993

[rclark72]

But, I do agree that this randomizer should be extracted and isolated.

[Serneum]

defmodule GamePicker.Randomizer do
  def new do
    :rand.seed(:exsplus, DateTime.utc_now |> DateTime.to_date |> Date.to_erl)
  end

  def new(seed) do
    :rand.seed(:exsplus, seed)
  end

  def randomize(games_list) do
    games_list
    |> Enum.take_random(1)
    |> List.first
  end
end

[mbramson]

Ha! I love this solution. It's hacky in a really elegant way.

[Serneum]

Tests will currently fail

[daveshah]

This won't work because new returns the result of the last called function. Not the Randomizer.

[Serneum]

Yeah, Eric had narrowed that down. I was still treating it like it was OO
On Tue, Oct 18, 2016 at 9:29 AM Dave Shah notifications@github.com wrote:

@daveshah commented on this pull request.

In test/nermesterts/game_picker_test.exs
#24 (review)
:

@@ -0,0 +1,15 @@
+defmodule GamePickerTest do

• use ExUnit.Case

• alias Nermesterts.Game
• alias GamePicker

• test "game selection is consistent when the day and number of players are the same" do
• games = [%Game{name: "Game1", min_players: 2, max_players: 4},

•         %Game{name: "Game2", min_players: 2, max_players: 4},
  

•         %Game{name: "Game3", min_players: 2, max_players: 4}]
  

• randomizer = GamePicker.Randomizer.new({2016, 10, 17})

This won't work because new returns the result of the last called
function. Not the Randomizer.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#24 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABCohOiXuS2uvudlkA4IN9_XCeanroc6ks5q1Mm4gaJpZM4KY-x6
.

[ericworkman]

Todo

• Rename randomizer
• Make the tests great again
• Refactor randomizer cause it ain't too pretty (and namespace it better)

In short, this randomizer uses the current year and day of year to as the index of the list of available games to reliably choose the same game for a given day and game list. I'm not happy with the tests, as I'm not too sure about how to set this up to be tested reliably (but I barrelled through with it anyway). Paired with @Serneum while doing this.