1

It seems to me that this is often encountered in practice and I was wondering if there is a design pattern for the following:

Suppose I have a class that represents a card:

public class Hand {
    private static Random rng;
    private Rank rank;
    private Suit suit;
    public Hand() {
        if (prng == null)
            prng = new Random();
        }
        //more stuff
    }
}

Every time I instantiate a Card, I want a random card. Doing it the above way wastes one comparison to null per instantiation. If I create a static field and allocate it in the deceleration:

private static Random rng = new Random();

a new Random object gets instantiated every time - also not very efficient.

If I pass the Random object into the constructor, I violate "tell, don't ask."

Perhaps there is another suggestion, but so far, I think the best way is the null check.

  • 5
    a new Random object gets instantiated every time - also not very efficient. that's incorrect, a static field will only be initialized once for all instances of that type. – Jeroen Vannevel Dec 29 '13 at 15:11
7

A constructor should be deterministic if possible, otherwise your code will be untestable. Have at least one overloaded constructor that can accept an RNG:

public Hand(Random prng) {
  // more stuff
}

An argument-less constructor could give a default RNG, as in your design sketch. While a null comparision isn't that expensive and a new Hand isn't that common, you could just initialize the RNG with the class:

private static Random prng = new Random();

public Hand() {
  this(prng);
}

This way, the RNG initialization is executed only once.

Otherwise, the quote about premature optimization applies…

  • 1
    However, be careful with adding functions just for testing. Where they replace other overloads, you stop testing the original function. It can be a challenge to keep your code coverage high. – MSalters Dec 30 '13 at 16:03
  • Make the constructor that takes a collaborator (the Random instance) protected and call it from your default constructor. Your unit test will need to be in the same package (which should be standard practice). This allows controlled reproducible unit testing and guarantees that the flow is the same in test as it is in production. – Rick Ryker Jul 3 '17 at 18:42
6

If I create a static field and allocate it in the deceleration:

private static Random rng = new Random();

a new Random object gets instantiated every time - also not very efficient.

This is simply not true - it will create a single Random when the class is initialized, that's what static means. Thus, it is the correct solution. But also note that Random objects are pretty lightweight and there is no reason to be overly reluctant to create them, unless you're doing it millions of times per second.

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.