HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Weighted Probabilities with Integers for Game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withweightedgameprobabilitiesforintegers

Problem

The idea here is to pick a skill at random for the player each round. Four skills will be chosen, they are all random, and the same skill can repeat multiple times.

The player has a skill level for each ability. The higher the skill level, the greater the chance that the skill will be chosen.

Rather than use floats and percentages, I have created this solution based on ranges and integers. The readability of the code is what I am concerned with here. Does it make sense from looking at it? Would more comments help, or should I use a different approach?

I am using this in a libGDX game, so I am limited to Java 6.

private Ability getRandomAbility() {
    final Map abilityRanges = new HashMap();
    int count = 0;
    for (Ability ability : this.skillLevels.keySet()) {
        int abilityLevel = this.skillLevels.get(ability);
        BZRange abilityRange = new BZRange(count, count + abilityLevel);
        abilityRanges.put(abilityRange, ability);
        count += abilityLevel;
        //increase the count by an extra one so that the ranges dont overlap
        count++;
    }

    int randomNumber = this.random.nextInt(count);
    for (BZRange range : abilityRanges.keySet()) {
        if (range.contains(randomNumber)) {
            return abilityRanges.get(range);
        }
    }

    return null;
}


Here is the BZRange class, it is very simple:

public class BZRange {

    private int low;
    private int high;

    public BZRange(int low, int high){
        this.low = low;
        this.high = high;
    }

    public boolean contains(int number){
        return (number >= this.low && number <= this.high);
    }
}

Solution

You can add the final modifier for your BZRange fields since this class is immutable - this makes the definition (slightly) clearer. :)

//increase the count by an extra one so that the ranges dont overlap


At first, I thought this was an odd comment to read, but then I realize this is just because your contains() implementation is a bound-inclusive comparison... If BZRange is only used purely for this computation, perhaps you can consider changing the contains() implementation to exclude the upper bound hmms?

As for the logic itself... I did a quick search and uncovered not one, but two Stackoverflow suggestions for using a TreeMap and its ceilingEntry() or floorEntry() methods, so you may want to consider that. The gist is:

private Ability getRandomAbility() {
    int offset = 0;
    TreeMap abilityMap = new TreeMap();
    for (Entry entry : this.skillLevels.entrySet()) {
        abilityMap.put((offset += entry.getValue()), entry.getKey());
    }
    // edit: added 1 offset as well for adjustment
    return abilityMap.ceilingEntry(1 + this.random.nextInt(offset)).getValue();
}

Code Snippets

//increase the count by an extra one so that the ranges dont overlap
private Ability getRandomAbility() {
    int offset = 0;
    TreeMap<Integer, Ability> abilityMap = new TreeMap<Integer, Ability>();
    for (Entry<Ability, Integer> entry : this.skillLevels.entrySet()) {
        abilityMap.put((offset += entry.getValue()), entry.getKey());
    }
    // edit: added 1 offset as well for adjustment
    return abilityMap.ceilingEntry(1 + this.random.nextInt(offset)).getValue();
}

Context

StackExchange Code Review Q#101919, answer score: 6

Revisions (0)

No revisions yet.