patternjavaMinor
Weighted Probabilities with Integers for Game
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.
Here is the
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
At first, I thought this was an odd comment to read, but then I realize this is just because your
As for the logic itself... I did a quick search and uncovered not one, but two Stackoverflow suggestions for using a
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 overlapAt 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 overlapprivate 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.