patternjavaModerate
Generating a not-the-same-as-last-time random number
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.
vs
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):
This contains a seemingly infinite loop.
The first block is better:
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
The 'while loop' solution would be more readable (even though it duplicates the
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
@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:
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.