patterncsharpMinor

Is this roulette selection extension method beautiful or disgusting?

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

thisdisgustingselectionmethodextensionroulettebeautiful

Problem

So I just wrote the following extension method to do roulette selection on a generic list. I'm specifically unsure of the use of the while loop. Is this beautiful or a disgusting mess?

// This is either yuck or beautiful
public static T rouletteSelect(this List list, Func fitnessFunc, double max)
{
    T selected;
    while (fitnessFunc(selected = list.getRandom()) / max <= Rand.NextDouble()) { }
    return selected;
}

Solution

I am on the it's disgusting side.

I usually think at extension methods as something that could be useful by themselves. In your case you need to provide some external context with max, which could lead to confusing code. Who should be in charge of updating it? In my opinion you should have something that has both the responsibilities of keeping track of the current max values and to select the next one.

Also, I'm not sure if you're talking about this selection method, but there they divide the fitness value of an item by the sum of the fitness values of all the items, not by a maximum value.

You cannot test it. The calls to list.getRandom() and Rand.NextDouble() make it impossible. I'd rather make the dependencies on random number generators explicit.

As a matter of style I don't like having an empty body for a loop.

Given that you want to implement the selection method I linked earlier, I'd probably go for another different approach. I'm not sure yours actually implements it (but I have not looked at it for long enough to be sure it doesn't work as you would expect to).

public static T RouletteSelect(this List list, Func fitnessFunction, double randomValue)
{
    var fitnessValues = list.Select(x => fitnessFunction(x));
    var totalFitness  = fitnessValues.Sum();
    var probabilities = fitnessValues.Select(x => x/totalFitness).ToList();
    var i = 0;
    var sum = probabilities[0];
    while(randomValue <= sum)
    {
      i = i + 1;
      sum += probabilities[i];
    }
    return list[i];
}

Code Snippets

public static T RouletteSelect<T>(this List<T> list, Func<T, double> fitnessFunction, double randomValue)
{
    var fitnessValues = list.Select(x => fitnessFunction(x));
    var totalFitness  = fitnessValues.Sum();
    var probabilities = fitnessValues.Select(x => x/totalFitness).ToList();
    var i = 0;
    var sum = probabilities[0];
    while(randomValue <= sum)
    {
      i = i + 1;
      sum += probabilities[i];
    }
    return list[i];
}

Context

StackExchange Code Review Q#59290, answer score: 3

Revisions (0)

No revisions yet.