Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
4  
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

2 Answers 2

up vote 6 down vote accepted

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…

share|improve this answer
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

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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