Generating Even Random Numbers

Question

I have this class which is used as part of a game. It needs to generate Random Even values, which is done by generating random numbers until the result is even.

Is there a better way to do this?

Also, I currently have the methods implemented as private instance methods, but should I declare generateRandomNumber() and evenNumber(number) as static methods? Will it have any benefits?

public class Game {
    //...

    public void opponentSaysEvenNumber() {
        int number = generateRandomEvenNumber();
        System.out.println("Opponent: " + number);
    }

    private int generateRandomEvenNumber() {
        Random random = new Random();
        int number = random.nextInt();
        while (!evenNumber(number)) {
            number = random.nextInt();
        }
        return number;
    }

    private boolean evenNumber(int number) {
        return (number % 2) == 0;
    }
}

Answer 1

Yes. I would go one step further and declare your functions as private static final, if possible. The combination of those three keywords means that the code would be unaffected by any instance variable, any superclass, or any subclass, and is also not callable by any code external to the class. Therefore, the compiler has enough of a hint that it could decide to inline the entire function.

I would also rename evenNumber(int number) to isEven(int number). There is a convention in Java that functions named isSomething() return a boolean and have no side effects. Your function meets those criteria.

To generate a random even number, you could just take random.nextInt() & -2 to mask off the least significant digit. That would be more efficient than looping, testing, and discarding. In that case, the whole question about helper functions would be irrelevant.

It's bad practice to create a new instance of Random every time you want to generate one random number, though. The pseudorandom number generator actually carries some state, even if you don't think of it that way. You should therefore use a private static variable to store the Random object.

Answer 2

Declaring such methods static can increase the readability of your code. For the reader it will be obvious that the method does not depend on the internal state of an instance of the class.

Answer 3

Your technique is inefficient. To generate even numbers, just do any of the following:

• Mask off the least significant bit (-2 is 0xFFFFFFFE, or a bitmask with all ones except for the last bit)

  random.nextInt() & -2;
  

• Shift left by one bit using the bit-shift operator

  random.nextInt() << 1;
  

• Shift left by one bit using multiplication

  random.nextInt() * 2;
  

Answer 4

If you don't use any of the attributes of the class, then it means that you should probably put this method somewhere else. Maybe in another class, a RandomEvenNumberGenerator for example.

I've always considered static a bad practice. It opens the path to so many problems, semantic issues and "I know where you live" syndrome, which in turns make testing harder, etc.

Answer 5

Why not just double the first random number generated and return it? You will still have random numbers, their distribution is not corrupted and its faster.

public class Game {
    //...

    public void opponentSaysEvenNumber() {
        int number = generateRandomEvenNumber();
        System.out.println("Opponent: " + number);
    }

    private int generateRandomEvenNumber() {
        Random random = new Random();
        int number = random.nextInt();
        return number * 2;
    }
}

Answer 6

Consider using a ThreadLocalRandom

private int generateRandomEvenNumber() {
    return ThreadLocalRandom.current().nextInt() & -2;
}

ThreadLocalRandom was added in Java 7 and solves a few performances issues with the Random object, especially in multi-threaded environments. Even if you're not in a multi-treaded environment, I think it is a good idea to use it every time. Furthermore the Api is nicer than the old Random one (you don't have to create the object, just use the static method).

Using & -2 (or any other method proposed by @wizzi poo) will ensure the generated number is always even by setting the last bit of the generated number to 0. This is better than doing a while as there is always the chance Random could never generate an even number (highly unlikely).

Answer 7

You do not want to mask to make your elements even. There are an odd number of positive and an odd number of negative values. This can throw you for a loop (no pun intended) by causing them to be slightly biased.

Instead, you want to provide a min and max and find an even number in that range. You want to do this by shifting (actually division and multiplication, but because we're dealing with a power of 2 we can do it with bit shifts) and not with rounding (which is what masking is essentially).

As a general rule of thumb, you don't want to throw away random bits when you don't have to.

private static final Random rand = new Random(); 

public static int randomEvenNumber(int min, int max) {
    if(min % 2 != 0) throw new IllegalArgumentException("Minimum value must be even");
    if(max % 2 != 0) throw new IllegalArgumentException("Maximum value must be even");
    int range = max - min;
    int rangeHalf = range >> 1; // divided by 2
    int randomValue = rand.nextInt(rangeHalf);
    randomValue = randomValue << 1; // multiplied by 2 to make it even
    return min + randomValue;
}

public static final int randomEvenNumber(int max) {
    return randomEvenNumber(0,max);
}

Answer 8

Perhaps a higher-level approach may get you to a better solution.

The random generator methods (generateRandomEvenNumber() and evenNumber()) are mathematical in nature and really not specific to the game. They have no dependencies on the game class (e.g. they don't use fields) and they have no side effects.

My approach would be to do something like this. Create a RandomService class with those methods and inject an instance of the service into the game. Using Spring or some other DI (dependency injection) framework, the service can be created as a singleton. The methods on RandomService can be public because they have no dependencies or side effects and they are easily testable, separately from the game.

Answer 9

Declaring a static and unique random number generator method is a good choice if you want to centralize the generation of those numbers. However, don't forget to also mark the method as synchronized if you're planning to run your code in a multithreaded environment.

You should also declare the Random instance as private final static.

Finally, as the evenNumber method consists of a single test, you could move that test to your generateRandomEvenNumber method and have your class composed by only two methods.

Another possibility if you want to centralize the generation of those random numbers is to create a Singleton random numbers generator.

Answer 10

Another option is to add or subtract 1 in case of odd random number.

...
number = random.nextInt();
if (number % 2 == 1) {
    number++;
    number %= MAX_NUMBER;   // assuming upper limit is required
}
...