2
\$\begingroup\$

Is there any ways I can write redundant-less code without this infinite loop? I have also tried putting the conditions and the generating number, but since it has two conditions, the output number will be different.

private static final int PERCENT_97 = 3;
/**
 * Returns returns a pseudo-random Gaussium number. Forumla:
 * {@code (RanGassium * 3) + mean }
 * 
 * @param mean
 *            mean of returning value
 * @param min
 *            min bound
 * @param max
 *            max bound
 * @return a pseduo-random Gaussium that is close to the mean.
 */
public static double getRandomGaussium(double mean, double min, double max) {
    double returningNumber = 0;
    double rangeMean=(max-min)/2;
    while (true) {
        returningNumber = (generator.nextGaussian() * rangeMean/PERCENT_97) + mean;
        if (returningNumber > min && returningNumber < max) {
            break;
        }
    }
    return returningNumber;
}
\$\endgroup\$
  • \$\begingroup\$ It's not infinite if you provide a way to escape the loop. That is, if the escape can be reached although. \$\endgroup\$ – Jonny Sooter Jun 19 '13 at 15:53
5
\$\begingroup\$

A do while would be an equivalent construct. I don't really think either is more or less preferable, but is an alternative that avoids the appearance of an infinite loop:

do {
    returningNumber = (generator.nextGaussian() * rangeMean/PERCENT_97) + mean;
} while (returningNumber < min || returningNumber > max);
return returningNumber;

That still leaves open a possibility of an eternal loop, though. You should probably at least validate your params to guard against that (eg., a min of 1 and max of 1 would run forever). You may also want to put an iteration counter to give up and throw an exception after so many iterations. At that point, a for loop with an early return if one is found may be a better choice than stuffing it all into the where or if conditionals:

for (int i = 0; i < MAX_TRIES; i++) {
    returningNumber = (generator.nextGaussian() * rangeMean/PERCENT_97) + mean;
    if (returningNumber > min && returningNumber < max) {
        return returningNumber;
    }
}
throw new IGiveUpException();
\$\endgroup\$
3
\$\begingroup\$

Firstly, I'm not convinced that the javadoc of you method matches what the code actually does. Certainly, the formula (RanGassium * 3) + mean needs some explanation ... because it does not match what the code is literally doing.

Second, you are missing some critical validation on the input parameters. For instance if you call the method with min greater than max you will get a real infinite loop.

Third, I'm not sure of your methodology for using min, max and PERCENT_97 to generate the multiplier.


Anyway, the away to avoid the loop is to take a look at the various methods listed in Wikipedia for generating Gaussian random numbers and picking one that allows you set the bounds on the input side of the uniform -> Gaussian mapping. The first bullet looks promising, though you'd need to understand the mathematics. A general approach based on this for generating Gaussian distributed PRNs in a range would be:

  • map min and max (in Gaussian space) to uniform space; i.e. calculate "phi(min)" and "phi(max)" (I think ...)

  • generate a single uniform number in the range "phi(min) < N < phi(max)"

  • calculate "phi-1(N)"

From a performance perspective, this is probably only worthwhile if min and max are very close together, and you would need to do a large number of iterations of your "potentially infinite" loop.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.