2
\$\begingroup\$

Yatzy

Yatzy is a dice rolling game where players aim to get particular die combinations.

This program counts probability of each such combinations. They are:

  1. All five ones,
  2. All five twos,
  3. Same for threes, fours, fives and sixes,
  4. One pair: two dice with the same number,
  5. Two pairs: just like in (4), but repeats on two different numbers,
  6. Three of a kind: three dice with the same number,
  7. Four of a kind: four dice with the same number,
  8. Small straight: numbers 1, 2, 3, 4, 5 present,
  9. Large straight: numbers 2, 3, 4, 5, 6 present.
  10. Full house: one pair and one triple,
  11. Yatzy: all five dice with the same number.

Code

package com.github.coderodde.prob.yatzy;

import java.util.Arrays;

/**
 *
 * @version 1.0.1 (Nov 2, 2024)
 * @since 1.0.0 (Nov 1, 2024)
 */
public final class YatzyProbabilityCounter {

    private static final int N = 6;
    
    private final int[] dice = new int[5];
    private final int[] counters = new int[N + 1];
    
    public YatzyProbabilityCounter() {
        for (int i = 0; i < dice.length; i++) {
            dice[i] = 1;
        }
    }
    
    public boolean inc() {
        for (int i = dice.length - 1; i >= 0; i--) {
            if (dice[i] < N) {
                dice[i]++;
                
                for (int j = i + 1; j < dice.length; j++) {
                    dice[j] = 1;
                }
                
                return true;
            }
        }
        
        return false;
    }
    
    public int getNumberOfYatzyConfigurations() {
        return (int) Math.pow(N, dice.length);
    }
    
    @Override
    public String toString() {
        final StringBuilder sb = new StringBuilder(dice.length);
        
        for (final int die : dice) {
            sb.append(die);
        }
        
        return sb.toString();
    }
    
    public static void main(String[] args) {
        final YatzyProbabilityCounter ypc = new YatzyProbabilityCounter();
        final int numberOfConfigurations = ypc.getNumberOfYatzyConfigurations();
        final String outputFormat = 
                String.format(
                        "%%%dd", 
                        Integer.toString(numberOfConfigurations).length());
        
        int diceCombinationNumber = 1;
        
        int ones = 0;
        int twos = 0;
        int threes = 0;
        int fours = 0;
        int fives = 0;
        int sixes = 0;
        
        int onePairs = 0;
        int twoPairs = 0;
        int threeOfKind = 0;
        int fourOfKind = 0;
        int smallStraights = 0;
        int largeStraights = 0;
        int fullHouses = 0;
        int yatzies = 0;
        
        do {
            final String diceCombinationNumberString = 
                    String.format(outputFormat,
                                  diceCombinationNumber);
                    
            System.out.printf("Dice combination %s: %s\n", 
                              diceCombinationNumberString, 
                              ypc.toString());
            
            diceCombinationNumber++;
            
            if (ypc.isOnes()) {
                ones++;
            } 
            
            if (ypc.isTwos()) {
                twos++;
            }
            
            if (ypc.isThrees()) {
                threes++;
            }
            
            if (ypc.isFours()) {
                fours++;
            }
            
            if (ypc.isFives()) {
                fives++;
            }
            
            if (ypc.isSixes()) {
                sixes++;
            }
            
            if (ypc.isOnePair()) {
                onePairs++;
            }
            
            if (ypc.isTwoPairs()) {
                twoPairs++;
            }
            
            if (ypc.isThreeOfKind()) {
                threeOfKind++;
            }
            
            if (ypc.isFourOfKind()) {
                fourOfKind++;
            }
            
            if (ypc.isSmallStraight()) {
                smallStraights++;
            }
            
            if (ypc.isLargeStraight()) {
                largeStraights++;
            }
            
            if (ypc.isFullHouse()) {
                fullHouses++;
            }
            
            if (ypc.isYatzy()) {
                yatzies++;
            }
        } while (ypc.inc());
        
        System.out.println("\nGenerated all the probabilities.\n");
        
        final Fractional fractionalOnes = 
                new Fractional(ones, numberOfConfigurations).gcd();
        
        final Fractional fractionalTwos = 
                new Fractional(twos, numberOfConfigurations).gcd();
        
        final Fractional fractionalThrees = 
                new Fractional(threes, numberOfConfigurations).gcd();
        
        final Fractional fractionalFours = 
                new Fractional(fours, numberOfConfigurations).gcd();
        
        final Fractional fractionalFives = 
                new Fractional(fives, numberOfConfigurations).gcd();
        
        final Fractional fractionalSixes = 
                new Fractional(sixes, numberOfConfigurations).gcd();
        
        final Fractional fractionalOnePair = 
                new Fractional(onePairs, numberOfConfigurations).gcd();
        
        final Fractional fractionalTwoPairs = 
                new Fractional(twoPairs, numberOfConfigurations).gcd();
        
        final Fractional fractionalThreeOfKind = 
                new Fractional(threeOfKind, numberOfConfigurations).gcd();
        
        final Fractional fractionalFourOfKind = 
                new Fractional(fourOfKind, numberOfConfigurations).gcd();
        
        final Fractional fractionalSmallStraights = 
                new Fractional(smallStraights, numberOfConfigurations).gcd();
        
        final Fractional fractionalLargeStraights = 
                new Fractional(largeStraights, numberOfConfigurations).gcd();
        
        final Fractional fractionalFullHouses = 
                new Fractional(fullHouses, numberOfConfigurations).gcd();
        
        final Fractional fractionalYatzies = 
                new Fractional(yatzies, numberOfConfigurations).gcd();
        
        //// Printing statistics:
        
        System.out.printf("Ones (all ones)    :   %s\n", fractionalOnes);
        System.out.printf("Twos (all twos)    :   %s\n", fractionalTwos);
        System.out.printf("Threes (all threes): %s\n", fractionalThrees);
        System.out.printf("Fours (all fours)  :  %s\n", fractionalFours);
        System.out.printf("Fives (all fives)  :  %s\n", fractionalFives);
        System.out.printf("Sixes (all sixes)  :  %s\n", fractionalSixes);
        
        System.out.println();
        
        System.out.printf("One pair (two same numbers):          %s\n", fractionalOnePair);
        System.out.printf("Two pairs (two different pairs):      %s\n", fractionalTwoPairs);
        System.out.printf("Three of a kind (three same numbers): %s\n", fractionalThreeOfKind);
        System.out.printf("Four of a kind (four same numbers):   %s\n", fractionalFourOfKind);
        System.out.printf("Small straights (1, 2, 3, 4, 5):      %s\n", fractionalSmallStraights);
        System.out.printf("Large straights (2, 3, 4, 5, 6):      %s\n", fractionalLargeStraights);
        System.out.printf("Full houses (one pair, one triple):   %s\n", fractionalFullHouses);
        System.out.printf("Yatzies (all same numbers):           %s\n", fractionalYatzies);
    }
    
    public boolean isOnes() {
        return isParticular(1);
    }
    
    public boolean isTwos() {
        return isParticular(2);
    }
    
    public boolean isThrees() {
        return isParticular(3);
    }
    
    public boolean isFours() {
        return isParticular(4);
    }
    
    public boolean isFives() {
        return isParticular(5);
    }
    
    public boolean isSixes() {
        return isParticular(6);
    }
    
    public boolean isOnePair() {
        loadCounterArray();
        
        for (final int c : counters) {
            if (c == 2) {
                return true;
            }
        }
        
        return false;
    }
    
    public boolean isTwoPairs() {
        loadCounterArray();
        
        int pairs = 0;
        
        for (final int c : counters) {
            if (c == 2) {
                pairs++;
            }
        }
        
        return pairs == 2;
    }
    
    public boolean isThreeOfKind() {
        loadCounterArray();
        
        for (final int c : counters) {
            if (c == 3) {
                return true;
            }
        }
        
        return false;
    }
    
    public boolean isFourOfKind() {
        loadCounterArray();
        
        for (final int c : counters) {
            if (c == 4) {
                return true;
            }
        }
        
        return false;
    }
    
    public boolean isSmallStraight() {
        loadCounterArray();
        
        for (int i = 1; i <= 5; i++) {
            final int c = counters[i];
            
            if (c != 1) {
                return false;
            }
        }
        
        return true;
    }
    
    public boolean isLargeStraight() {
        loadCounterArray();
        
        for (int i = 2; i <= 6; i++) {
            final int c = counters[i];
            
            if (c != 1) {
                return false;
            }
        }
        
        return true;
    }
    
    public boolean isFullHouse() {
        loadCounterArray();
        
        boolean two = false;
        boolean three = false;
        
        for (final int c : counters) {
            if (c == 2) {
                two = true;
            } else if (c == 3) {
                three = true;
            }
        }
        
        return two && three;
    }
    
    public boolean isYatzy() {
        loadCounterArray();
        
        final int firstDie = dice[0];
        
        for (int i = 1; i < dice.length; i++) {
            if (firstDie != dice[i]) {
                return false;
            }
        }
        
        return true;
    }
    
    private void loadCounterArray() {
        Arrays.fill(counters, 0);
        
        for (final int d : dice) {
            counters[d]++;
        }
    }
    
    private boolean isParticular(final int die) {
        for (final int d : dice) {
            if (d != die) {
                return false;
            }
        }
        
        return true;
    }
}

final class Fractional {

    private final int numerator;
    private final int denominator;
    
    Fractional(final int numerator, final int denominator) {
        this.numerator = numerator;
        this.denominator = denominator;
    }
    
    double asDouble() {
        return ((double) numerator) / ((double) denominator);
    }
    
    Fractional gcd() {
        int a = numerator;
        int b = denominator;
        
        while (b != 0) {
            final int t = b;
            b = a % b;
            a = t;
        }
        
        return new Fractional(numerator / a,
                              denominator / a);
    }
    
    @Override
    public String toString() {
        return numerator + " / " + denominator + " = " + asDouble();
    }
}

... and it outputs:

Line    1: 11111
Line    2: 11112
Line    3: 11113
Line    4: 11114
Line    5: 11115
Line    6: 11116
Line    7: 11121
.
.
.
Line 7770: 66656
Line 7771: 66661
Line 7772: 66662
Line 7773: 66663
Line 7774: 66664
Line 7775: 66665
Line 7776: 66666

Generated all the probabilities.

Ones:   1 / 7776 = 1.286008230452675E-4
Twos:   1 / 7776 = 1.286008230452675E-4
Threes: 1 / 7776 = 1.286008230452675E-4
Fours:  1 / 7776 = 1.286008230452675E-4
Fives:  1 / 7776 = 1.286008230452675E-4
Sixes:  1 / 7776 = 1.286008230452675E-4

One pair:        475 / 648 = 0.7330246913580247
Two pairs:       25 / 108 = 0.23148148148148148
Three of a kind: 125 / 648 = 0.19290123456790123
Four of a kind:  25 / 1296 = 0.019290123456790122
Small straights: 5 / 324 = 0.015432098765432098
Large straights: 5 / 324 = 0.015432098765432098
Full houses:     25 / 648 = 0.038580246913580245
Yatzies:         1 / 1296 = 7.716049382716049E-4

Critique request

I would like to receive any commentary regarding my work.

\$\endgroup\$
6
  • 4
    \$\begingroup\$ Suggestion regarding the output: Line 1: 11111 -> Dice combination 1: 11111, things like One pair: 475 / 648, Large straights: 5 / 324 would benefit from a header clarifying what these fractions mean. \$\endgroup\$ Commented Nov 1, 2024 at 12:07
  • 1
    \$\begingroup\$ @AlexanderIvanchenko I have updated my code according to your comments. \$\endgroup\$ Commented Nov 2, 2024 at 9:15
  • \$\begingroup\$ Looks better (and I gave you my vote). But the output might still look a bit puzzling for a reader. Because combinations are labeled as lines, and it's not immediately clear that fractions are reduced (I understand that you generated all 6^5 possible dice combinations and calculated how many of them much each criterion). \$\endgroup\$ Commented Nov 2, 2024 at 10:28
  • 1
    \$\begingroup\$ By the way, yatzy-combination described twice: "1) All five ones, 2) All five twos, 3) Same for threes, fours, fives and sixes" and "11) Yatzy: all five dice with the same number." \$\endgroup\$ Commented Nov 2, 2024 at 13:07
  • 1
    \$\begingroup\$ I posted a question with similar theme some time ago. You may find inspiration on how to make the code more modular: codereview.stackexchange.com/questions/288807/… \$\endgroup\$ Commented Nov 3, 2024 at 7:49

1 Answer 1

Reset to default
4
\$\begingroup\$

Mathematical correctness

There's an inconsistency in a way the combinations are treated.

According to the logic you presented, a Full house is also considered to be a One pair combination and a Three of a kind combination. Similarly, a Two pairs combination is also considered to be a One pair combination.

If the goal was to calculate the probability of receiving a certain bonus in a game of Yatzy, then the end results are incorrect because when a player rolls a Full house they only get one bonus for this combination, not 3 bonuses (for Full house, One pair and Three of a kind).

And if you were aiming to calculate the probabilities of rolling two of five fair dice with the same score, three of five dice with the same score, etc. Then the results are not correct either. Because, for instance, Three of a kind, Four of a king and Yatzy combinations are not treated as a One pair (although, there's at least one pair of dice with the same score in each of them).

Naming

YatzyProbabilityCounter isn't really doing what it claims to do. It represents a dice combination, hence a more appropriate name for it would be YatzyCombination or YatzyDiceCombination.

All the job related to the probability calculations is happening in the main() method, that's a static routine which you can plug anywhere.

It's not immediately clear what N field in YatzyProbabilityCounter is supposed to represent. It should be something along the lines of MAX_DIE_SCORE.

Method is isParticular(int) is a bit vague, allDiceHaveScore(int) will communicate its purpose more clearly.

Method inc() is responsible for generating a new combination based on this combination. And the name doesn't communicate this intent well.

You're playing Mutable in a Wrong way

As I said, YatzyProbabilityCounter in fact describes a dice combination. And it's method inc() performs transition between two combinations, updating the state.

The issue is that it performs only a partial update, leaving the object in an inconsistent state with stale data related to the previous combination in the counters array.

To mitigate this, methods isThreeOfKind(), isFullHouse(), isYatzy(), etc. are invoking loadCounterArray() before proceeding with their logic, which is an SRP violation.

This approach is error-prone (you might have forgotten to invoke it in one of these 7 methods) and while calculating probabilities you're repeatedly "updating" already updated counters array by the same values for no good reason.

It wouldn't happen if method inc() was properly designed. It should update both dice and counters.

And it shouldn't be named inc() to begin with (something like nextCombination() will do better). Names matter. You can give a good name to a code element only when you understood well its responsibility, and if you understand the responsibility you mastered the power and can question the correctness of your design.

Similarly, constructor should produce a new combination instance with the fully initialized state (when you construct a new YatzyProbabilityCounter its counters are not evaluated, which should not be the case).

Refactoring suggestions

Firstly, I encourage you to implement suggestions regarding naming and state-management provided above.

If your intent was to calculate the probability of rolling a specific dice combination in a game of Yatzy, which I believe is the case. And a Full house is not supposed to be also considered a One pair and a Three of a kind (i.e. each combination can be characterized by only one type). Then it makes sense to introduce an attribute describing a combination type in the YatzyCombination.

The advantages of doing so:

  1. It will allow simplifying the class API by hiding checks isYatzy(), isFullHouse(), etc. by making them private and exposing instead an accessor method returning a combination type.
  2. Making methods isYatzy(), isFullHouse(), etc. internal, provides a safety net while still keeping their logic simple. For instance, isOnePair() might not account for other combinations that contain one pairs (as in your code), the correctness of determining the type of combination will be insured by the order in which these checks are performed. Which should happen in one place, in an internal dedicated method that you can thoroughly unit-test.

Additionally, implementation of combination type checks can be simplified by leveraging Stream API, e.g.

private boolean isYatzy() {
    return Arrays.stream(dice).allMatch(d -> d == dice[0]);
}

Remainder: updating counters array is not a responsibility of this method, as explained above it should happen when transitioning to the next combination, not while identifying the combination type.

Now let me outline how YatzyCombination class might look like:

public final class YatzyCombination {
    
    public enum CombinationType {
        YATZY, FOUR_OF_KIND, THREE_OF_KIND, TWO_PAIRS, ONE_PAIR,
        SMALL_STRAIGHT, LARGE_STRAIGHT, FULL_HOUSE, CHANCE
    }
    
    private static final int MAX_DIE_SCORE = 6;
    private static final int DICE_NUMBER = 5;
    private final int[] dice = new int[DICE_NUMBER];
    private final int[] counters = new int[MAX_DIE_SCORE + 1];
    private CombinationType type;
    
    public YatzyCombination() {
        Arrays.fill(dice, 1);
        this.counters[1] = DICE_NUMBER;
        this.type = YATZY;
    }
    
    public boolean nextCombination() {
        if (tryUpdateDice()) {       // logic from inc() method
            updateCounters();        // logic from loadCounterArray() method
            updateCombinationType();
            return true;
        }
        return false;
    }
    
    // other methods
}

Where method updateCombinationType() can be implemented as a chain of if-statement performing checks isYatzy(), isFullHouse(), isLargeStraight(), etc. in the order that avoids ambiguity and returning a CombinationType that corresponds to the first matching condition.

Here's a couple observations on further improvement:

  • Introducing a "Named constructor"

The no-args constructor exposed by this class creates an instance describing a 11111 yatzy-combination, but new YatzyCombination() doesn't communicate this business intent to the code reader. As an alternative option, we can introduce a static method YatzyCombination.allOnes() and make this constructor private.

Such methods are sometimes called named constructors because they produce an instance of a specific type with certain properties reflected in the method name (as opposed to factory methods, which return an instance of super-type).

Please, don't conflate with CQRS architectural pattern

This principle states that, ideally, a method should either be a query (return a value) or a command (perform a side-effect). Which to some degree overlaps with SRP.

Note that as every other design principle, it's not applicable everywhere. In some cases, following SRP can lead to cleaner design and simpler tests. But there are cases where applying it doesn't make sense (working with concurrency, building fluent APIs), or doesn't bring much value.

In the code shown above, I preserved the contract of your inc() method, which is superseded by nextCombination(). It both changes stuff and returns a boolean value.

In my opinion, in this case, it's not a big deal, and we can leave it like this. Although, seeing while (combination.nextCombination()) will make me pause and bring the method's documentation on the screen.

And it's a fairly simple example to introduce the concept. Here's what we'll get if we split the current implementation into command and query:

public void nextCombination() {
    if (!hasNextCombination()) {
        throw new IllegalStateException();
    }
    updateDice();           // only updates state and doesn't return anything
    updateCounters();
    updateCombinationType();
}

public boolean hasNextCombination() {
    return Arrays.stream(dice).anyMatch(d -> d != MAX_DIE_SCORE);
}
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.