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

Generating a not-the-same-as-last-time random number

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

Problem

I have to generate random number which differs from the number generated last time.

Which of these two variants is better? Or maybe you can suggest better implementation.

int generateRandom() {
    int randomNumber;
    do {
        randomNumber = random.nextInt(UPPER_BOUND);
    } while (randomNumber == lastRandomNumber);
    lastRandomNumber = randomNumber;
    return randomNumber;
}


vs

int generateRandom() {
    while (true) {
        int randomNumber = random.nextInt(UPPER_BOUND);
        if (randomNumber != lastRandomNumber) {
            lastRandomNumber = randomNumber;
            return randomNumber;
        }
    }
}

Solution

Neither option is great. Looking at the second option first (because it is the worst, in my opinion):

int generateRandom() {
    while (true) {
        int randomNumber = random.nextInt(UPPER_BOUND);
        if (randomNumber != lastRandomNumber) {
            lastRandomNumber = randomNumber;
            return randomNumber;
        }
    }
}


This contains a seemingly infinite loop. while(true) is often an indication that there's a better way to do things (not necessarily always, but in this case, yes). The reason it is a problem here is because it gives the impression that the loop repeats often, when, in fact, it seldom repeats ever.

The first block is better:

int generateRandom() {
    int randomNumber;
    do {
        randomNumber = random.nextInt(UPPER_BOUND);
    } while (randomNumber == lastRandomNumber);
    lastRandomNumber = randomNumber;
    return randomNumber;
}


This is better because the loop-terminating condition is much cleaner, more visible. The logic-path through the method is traditional. Still, it's not great. I dislike the need to declare the randomNumber variable as a separate declaration.

The 'while loop' solution would be more readable (even though it duplicates the nextInt() call), as:

int generateRandom() {
    int randomNumber = random.nextInt(UPPER_BOUND);
    while (randomNumber == lastRandomNumber) {
        randomNumber = random.nextInt(UPPER_BOUND);
    }
    lastRandomNumber = randomNumber;
    return randomNumber;
}


Even with the code duplication, it makes it clear that the normal case is handled, and then the while loop is a recovery process.

@Pimgd suggested sending the lastRandomNumber in as a parameter, and @maartinus suggested a single if-block condition rather than a while loop solution. In principle these are both good ideas. There is a way to make it completely reentrant, and also a way to remove the while-loop and the id-conditionals. My solution would be:

int generateRandom(int lastRandomNumber) {

    // add-and-wrap another random number to produce a guaranteed
    // different result.
    // note the one-less-than UPPER_BOUND input
    int rotate = 1 + random.nextInt(UPPER_BOUND - 1);
    // 'rotate' the last number
    return (lastRandomNumber + rotate) % UPPER_BOUND;

}


@mjolka pointed out that if UPPER_BOUND is large, it is possible for the values to overflow in the sum, and that the better solution would be:

return ((lastRandomNumber + rotate) & Integer.MAX_VALUE) % UPPER_BOUND;

Code Snippets

int generateRandom() {
    while (true) {
        int randomNumber = random.nextInt(UPPER_BOUND);
        if (randomNumber != lastRandomNumber) {
            lastRandomNumber = randomNumber;
            return randomNumber;
        }
    }
}
int generateRandom() {
    int randomNumber;
    do {
        randomNumber = random.nextInt(UPPER_BOUND);
    } while (randomNumber == lastRandomNumber);
    lastRandomNumber = randomNumber;
    return randomNumber;
}
int generateRandom() {
    int randomNumber = random.nextInt(UPPER_BOUND);
    while (randomNumber == lastRandomNumber) {
        randomNumber = random.nextInt(UPPER_BOUND);
    }
    lastRandomNumber = randomNumber;
    return randomNumber;
}
int generateRandom(int lastRandomNumber) {

    // add-and-wrap another random number to produce a guaranteed
    // different result.
    // note the one-less-than UPPER_BOUND input
    int rotate = 1 + random.nextInt(UPPER_BOUND - 1);
    // 'rotate' the last number
    return (lastRandomNumber + rotate) % UPPER_BOUND;

}
return ((lastRandomNumber + rotate) & Integer.MAX_VALUE) % UPPER_BOUND;

Context

StackExchange Code Review Q#66623, answer score: 13

Revisions (0)

No revisions yet.