Follow up Implementation on Monty Hall Simulation

Question

I have taken your inputs on my original question and formed a new implementation of the Monty Hall Simulation:

Again keep an eye on CleanCode and OOP

The Price to be won:

enum Price {
    GOAT, CAR
}

The Door keeping that price in secret

class Door {

    private final Price price;

    Door(Price price){
        this.price = price;
    }

    Price getPrice() {
        return price;
    }
}

the Player who participates in an Monty Hall Game

class Player {

    private final boolean preferresChange;
    private int winCounter;

    Player(boolean preferresChange){
        this.preferresChange = preferresChange;
    }

    boolean preferresChange(){
        return preferresChange;
    }

    void increaseWinCounter() {
        winCounter = winCounter + 1;
    }

    int getWinCounter(){
        return winCounter;
    }
}

the MontyHall in which the player takes action

class MontyHall {

    private final List<Door> doors;
    private Door selected;
    private Door openOne;
    private final Random random;
    private final int amountOfDoors;

    MontyHall(int seed, int amount) {
        random = new Random(seed);
        amountOfDoors = amount;
        doors = IntStream.range(0, amountOfDoors).
            mapToObj(e -> new Door(Price.GOAT)).
            collect(Collectors.toList());
        doors.set(random.nextInt(amountOfDoors), new Door(Price.CAR));
    }

    void chooseDoor(){
        selected = doors.get(random.nextInt(amountOfDoors));
    }

    void openDoor(){
        openOne = doors.stream().
            filter(d -> !d.equals(selected) && Price.GOAT == d.getPrice()).
            collect(Collectors.toList()).
            get(0);
    }

    void changeDoor(){
        selected = doors.stream().
            filter(d -> !d.equals(openOne) && !d.equals(selected)).
            collect(Collectors.toList()).
            get(0);
    }

    Price getPrice(){
        return selected.getPrice();
    }

}

and the simulator who hosts the games

class Simulator {

    public static void main(String[] args) {
        Simulator simulator = new Simulator();
        simulator.simulate(10000, 3);
    }

    private void simulate(int iterations, int amountDoors) {
        Player changer = new Player(true);
        Player stayer = new Player(false);

        Random random = new Random();
        for (int i = 0; i < iterations; i ++){
            int seed = random.nextInt();
            playOneRound(changer, seed, amountDoors);
            playOneRound(stayer, seed, amountDoors);
        }
        System.out.println("changer: "+changer.getWinCounter());
        System.out.println("stayer : "+stayer.getWinCounter());
    }

    private void playOneRound(Player player, int seed, int amountDoors) {
        playOneRound(player, new MontyHall(seed, amountDoors));
    }

    private void playOneRound(Player player, MontyHall montyHall) {
        montyHall.chooseDoor();
        montyHall.openDoor();
        if (player.preferresChange()){
            montyHall.changeDoor();
        }
        if (Price.CAR == montyHall.getPrice()){
            player.increaseWinCounter();
        }
    }

}

Answer 1

Positive Feedback

I liked how you follow the rules of Clean Code, that you write small methods and small classes and give all readable names!

You've introduced your own data types that make the code more readable. The MontyHall-Algorithm can now be read like a poem:

montyHall.chooseDoor();
montyHall.openDoor();
if (player.preferresChange()){
   montyHall.changeDoor();
}
if (Price.CAR == montyHall.getPrice()){
   player.increaseWinCounter();
}

And all your methods have only one intention level what makes them so easy to understand.

Some Criticism

Class Names Refer to Unexpected Behavior

I read on Wikipedia:

You [the player] pick a door, say No. 1, and the host, who knows what's behind the doors, opens another door, say No. 3

After I read your code I was a little bit confused at some places. But not because of the quality of your code, which is for me very high, but because of the quote above.

In your game the Player can't pick a Door. Instead the Player gets picked a door by MontyHall: (montyHall.chooseDoor()).

Additional to that the Player has a winCounter, but actually the Simulation should track how often a Player wins the MontyHall.

No Outcome

What would you expect your main to do without any knowledge about our code base?

public static void main(String[] args) {
    Simulator simulator = new Simulator();
    simulator.simulate(10000, 3);
}

It could run something like a game loop or start a GUI to interact with but instead it writes data to the console.

Much cleaner would be:

public static void main(String[] args) {
    Simulator simulator = new Simulator();
    simulator.printSimulations(10000, 3);
}

But now imagine you want to compare a "Changer" and a "Stayer". You can't do it because you have no outcome! You through your outcome away by printing it to the console instead of storing and returning it.

public static void main(String[] args) {
    Simulator simulator = new Simulator();
    SimulationResult result = simulator.simulate(10000, 3);
    int average = result.calculateAverageOfStayer();
    System.out.println(average);
}

Feature Envy

A method accesses the data of another object more than its own data.

The method playOneRound in playOneRound is in my eyes a feature envy. All the tasks should be done by MontyHall. So the method playOneRound should look like:

private void playOneRound(Player player, MontyHall montyHall) {
    montyHall.playGameWith(player);
}

Use Methods instead of Getters and Operations

In your code I can find code structure like x.getValue == something, but much better to read would be x.isSomething()

Some inspiration for you:

Price.GOAT == d.getPrice() means door.isHidingGoat()

Price.CAR == montyHall.getPrice() means montyHall.isWon()

!d.equals(selected) means door.notEquals(selected)

---

In your older post @Simon Forsberg mentioned:

I have a feeling that using a class for StraightPlayer, ChangePlayer and ShowMaster will be a bit overkill.

I know you still have the flag argument in Player. But that's fine. In general, refactoring can be an overkill at all. And you can see that for this little problem you can divide the logic into so many different classes and methods. When you write code, you have to find a balance between all OOP/clean code and your task.

Thanks for this cool problem and that you let me review it. I learned a lot while read your code :]

Answer 2

enum Price {

Is this what you meant? Why is it a price? Price for what? This would make much more sense if it were a prize.

        montyHall.chooseDoor();
        montyHall.openDoor();
        if (player.preferresChange()){
            montyHall.changeDoor();
        }

This seems incorrect. Monty doesn't choose the door; the player does. Monty opens the door. The player chooses and potentially changes that choice. And preferresChange is misspelled. It should be prefersChange.

In the simulation, Let's Make a Deal is the name of the show. Monty Hall is the host. So the class that you are calling MontyHall should be named LetsMakeADeal or simply GameShow.

You are hard coding some things. For example, your simulation is the classic Marilyn vos Savant one where Monty always opens a door and never opens the door with a car. That is a strategy. You should probably code that as a particular implementation. There are other possible strategies that will produce different results. For example, what if Monty only ever offers the choice when the player has selected the door with the car behind it? What if Monty can open the door and display the car? These strategies will change the results.