Multifunctional Monty Hall Simulator

Question

Based on this question on math.SE regarding probabilities in variations on the Monty Hall problem, I cobbled up a simulator in Ruby to give myself an introduction to the language.

Since this is my first Ruby program, I'm especially looking for feedback regarding Ruby style, but I would love any general code improvement suggestions as well (all the results seem correct, so I think the math is good). I'm more comfortable with Python and shell scripting, so I'm sure my preferences show through here.

Current functionality:

The program allows simulations of the following scenarios:

1. The host (Monty Hall) opens all but 1 door and the player switches to the door Monty left closed (the classic version)
2. Monty opens 1 extra door, leaving the rest closed, and you decide to switch to a different closed door (the premise of the math.SE question)
3. You pick a door and don't switch no matter what Monty does

Currently, it's hard-coded to simulate scenario #2 with 5 doors 1000 times, but that can be changed by switching the comment on the second-to-last line.

puts "Hello Monty!"

$wins = 0.0
$goats = 0.0

def choose_doors(num_doors)
  door_range = (1..num_doors)

  winning_door = rand(door_range)
  player_choice = rand(door_range)

  losing_doors = door_range.to_a - [winning_door] - [player_choice]
  monty_open = losing_doors.sample  # used when Monty opens 1 non-winning door

  return player_choice, monty_open, winning_door
end

def switch_door_multiple(choice, revealed, num_doors)
  # Monty revealed only 1 other door
  door_range = (1..num_doors)
  door_choices = door_range.to_a - [choice] - [revealed]

  new_choice = door_choices.sample
end

def count_result(final_choice, winning_door)
  if final_choice == winning_door
    $wins += 1
  else
    $goats += 1
  end
end

def simulate(num_doors, runs, strategy)
  for i in 1..runs
    player_door, open_door, winning_door = choose_doors(num_doors)
    if strategy == 1 # switch, 1 choice
      if player_door != winning_door
        player_door = winning_door
      else
        player_door = -1
      end
    elsif strategy == 2 # switch, multiple choice
      player_door = switch_door_multiple(player_door, open_door, num_doors)
    else
      # keep initial door
    end
    count_result(player_door, winning_door)
  end

  puts "wins: " + $wins.to_s
  puts "goats: " + $goats.to_s
  puts "win percentage: " + (100 * ($wins / ($wins + $goats))).to_s + "%"
end

def prompt_user
  print "Number of doors: "
  input_num_doors = gets.chomp.to_i
  while input_num_doors < 3
    puts "This only works with 3 or more doors"
    print "Number of doors: "
    input_num_doors = gets.chomp.to_i
  end

  print "Number of runs: "
  input_runs = gets.chomp.to_i
  while input_runs < 1
    puts "Need 1 or more runs"
    print "Number of runs: "
    input_runs = gets.chomp.to_i
  end

  puts "Simulation: "
  puts "[1]: Monty opens all but 1 door and you decide to switch"
  puts "[2]: Monty opens 1 door and you switch to a different closed door" 
  puts "[3]: Keep your door - you don't switch no matter what Monty does"
  print "Choose a game type (1/2/3): "
  input_strategy = gets.chomp.to_i

  while ![1, 2, 3].include? input_strategy
    print "Choose a game type (1/2/3): "
    input_strategy = gets.chomp.to_i
  end

  return input_num_doors, input_runs, input_strategy
end

# num_doors, num_runs, strategy = prompt_user
num_doors, num_runs, strategy = 5, 1000, 2

simulate(num_doors, num_runs, strategy)

Answer 1

Style is generally ok, but I'd recommend a more object-oriented approach - especially if you want to learn Ruby, since Ruby is thoroughly object-oriented.

Things I noticed:

1. Global variables (apart from built-in ones like $ARGV) are rare in Ruby. They're rare because you'll usually want to encapsulate your data in objects.

2. No need to write return at the end of methods; the result of the last expression is always implicitly returned.

3. You've quite a few methods that take 3 arguments, and some that return tuples. This is a general code-smell, in my opinion, because it's not very descriptive. Long parameter lists aren't easy to remember or parse, and multiple return values are also a fairly opaque data structure.

4. Overall structure is indeed very script'y and procedural.

Basically, you're passing all your data around, letting the methods modify them. A bit of basic object modelling could make your code better by encapsulation all that.

For instance, here's one way to model the game itself (not the full simulation). Just to give you something to chew on.

# A really simple Door class
class Door
  def initialize
    @open = false
  end

  def open?
    @open
  end

  def open!
    @open = true
  end
end

# This class models 1 instance/round of the game
class Game
  attr_reader :doors

  # Init a new game with +door_count+ doors, and randomly
  # picks one of them to be the winning door.
  # Raises an exception if door count is less than 3
  def initialize(door_count)
    raise ArgumentError, "There must be at least 3 doors" if door_count.to_i < 3
    @doors = Array.new(door_count.to_i) { Door.new }
    @winning_door = @doors.sample
  end

  # Set a random door as the player's pick
  # Raises an exception if there are no doors left to pick
  def pick_door
    raise "No doors to pick" if pickable_doors.empty?
    @picked_door = pickable_doors.sample
  end

  # open +count+ number of doors
  def open_doors(count)
    shuffled = openable_doors.shuffle
    [count, shuffled.count].min.times do 
      shuffled.pop.open!
    end
  end

  # Returns true if the player's pick is the winning door
  def won?
    @picked_door == @winning_door
  end

  # Array of doors that an be picked by the player
  def pickable_doors
    closed_doors - [@picked_door]
  end

  # Array of doors that can be safely opened by Monty
  def openable_doors
    closed_doors - [@picked_door, @winning_door]
  end

  # Array of closed doors
  def closed_doors
    @doors.reject(&:open?)
  end
end

The Door class may seem sort of pointless (and indeed there are many ways to do things), but instead of using, say, an array of plain booleans, an array of door objects allows us to reference specific doors. Not as array indices, but as the doors themselves.

Again, I'm not saying this is the only - or even a particularly great - way of doing things. But it reads pretty well, and is easy to use.

For instance, this code will play a basic game with 3 doors, of which 1 will be revealed, and the player changes their pick:

game = Game.new(3)
game.pick_door     # initial pick
game.open_doors(1) # reveal what's behind a door
game.pick_door     # change pick
game.won?          # => (~67% chance of a win)

You could make classes for Choice, Prize, or, heck, even Goat, if you want to completely model everything. But I'd at least make a SimulationRun (or similar) class.

This would encapsulate the notion of "a run" as in "N rounds of the game", and keep state out of the global scope.

I'd also consider modelling the actors (the player and Monty), and inject them into the simulation.

For instance, imagine being able to say something like:

sim = Simulation.new(3, Monty::Classic, Player::Fickle)
win_ratio = sim.run(1000)

i.e. create a simulation using 3 doors, run it 1000 times, using the "classic" Monty rules that leave two doors unrevealed, and a player that'll always pick a different door second time around.

Monty and the player could be classes or simply lambdas. For instance, the player can simply be a lambda that returns true/false when asked whether to change the pick. And "the Monty" could simply return the number of doors to open. Or you could make them a lot more complex, letting the player's reaction depend on, say, the phase of the Moon or something.

Again, I'm not saying that the above is the only way - or even a particularly great way - to do things, but I'd certainly encourage you to try doing object modelling of some kind.

All of this is of course presuming you don't just want to do the math, but actually want to be empirical :)

Answer 2

Some of your functions do not do what they describe - choose_doors does not return a choice of doors, or even records some doors as "chosen", it returns a whole boatload of things that aren't even comprehensible. What is a monty_open? Use more descriptive names.

More specifically, functions are almost always verbs or imperative statements. So when we see a function named switch_door_multiple, we assume that it takes a door_multiple and switches it - NOT that it switches you between multiple doors.

In places where you have simple if/else statements, like this:

if player_door != winning_door
  player_door = winning_door
else
  player_door = -1
end

Try using ternary statements to be more succint. The above is exactly equivalent to:

player_door = player_door == winning_door ? -1 : winning_door

Even if you don't use ternary statements you have a very simple if...!condition. Use unless instead.

In simulate you have the loop

for i in 1..runs
  ...
end

This might be a matter of opinion but a more "Ruby" way of doing that would be

runs.times do |run|
  ...
end

You repeat yourself a lot in prompt_user. Write some function prompt_for_number similar to this:

def prompt_for_number(prompt, minimum)
  until satisfied
    print prompt
    input = gets.chomp.to_i
    satisfied = true unless input < minimum
  end
  input
end

Then you can just call

num_doors = prompt_for_number("Number of doors (3 minimum): ", 3)
num_runs = prompt_for_number("How many runs? 1 minimum: ", 1)

Or something along those lines. I would actually change that prompt so that all it really took was something like

num_doors = prompt_for_number("doors", 3)

To avoid repeating myself.