Creating unique registration numbers using object factories

Question

I'm learning about immutability and uniqueness in Java, particularly the use of defensive programming and object factories.

I have been asked to create a class containing an object factory that when called creates a unique registration number comprised of a single letter and 4 digits. I think my solution ticks all boxes, but would appreciate some input on this one before I go any further to see if I'm on the right track.

public class RegistrationNumber
{
    private static final Map<String, RegistrationNumber> REG = new HashMap<String, RegistrationNumber>();  

    private static int number;
    private static char letter;
    private static String strRep;

    private RegistrationNumber(int number, char letter, String strRep) {

      this.number = number; 
      this.letter = letter;
      this.strRep = strRep;

    }      


    public static final RegistrationNumber getInstance() {

       Random r = new Random();

       int numbers = r.nextInt(9000) + 1000;

       Character letter = (char)(r.nextInt(26) + 'a');

       String strRep = letter + "" + numbers;

       RegistrationNumber n = REG.get(strRep);

       if (n == null) {

           n = new RegistrationNumber(numbers, letter, strRep);

           REG.put(strRep, n);

        }
             return n;
           }


      char getLetter() {

          return letter;

        }

      int getNumbers() {

          return number;

        }


      public String toString() {

       return strRep;

    }

}

Answer 1

The number, letter, and strRep fields should be final and must not be static. By making them static, you've made all of your RegistrationNumber objects very weird, and quite the opposite of immutable. Constructing one RegistrationNumber changes every previously generated RegistrationNumber!

getInstance() is a bit odd. Conventionally, the name getInstance() would retrieve a singleton instance — it should the same object every time it is called. What you have instead is a method that usually generates a new object, but occasionally returns an existing object. I'm puzzled by the motivation behind that behaviour.

Answer 2

Let's look at the requirements, an immutable unique Registration number which consists of a letter and 4 digits, constucted by a factory method. There are 4 parts here:

1. immuatable
2. letter/number combination
3. unique
4. factory.

Immutable

This is easy to solve, but harder to describe. A Java class that is final, has only private and final fields, is Immutable. In our case:

public final RegistrationNumber {
    private final String number;

    public RegistrationNumber(String number) {
        this.number = number;
    }

    public String getNumber() {
        return number;
    }
}

Letter/number

The letter/number combination makes things interesting. You have chosen to use random values. There is nothing in the spec about that. I think it would be simpler to use sequential values. Also, 0 is a number, what's wrong with A0001 as a value?

Here's a system that will produce a letter/number combination:

int currentNumber = 0;
int currentChar = 'A';

return String.format("%s%04d", currentChar, currentNumber);

That will 0-pad numbers less than 1000 to make them 4-digits wide.

Unique and factory

The factory is what makes the unique number, so you have to describe them together.

Also, a factory method means the constructor for the RegistrationNumber should be private.

A factory method would look like:

private static int count = 0;

public Registration newInstance() {
    count++;
    int val = count % 10000;
    char letter = (char)('a' + (count / 10000));
    return new RegistrationNumber(String.format("%s%04d", letter, val));
}

You may want to consider a thread-safe alternative, using an AtomicInteger instead of a plain int.

Answer 3

Adding to the previous good reviews which left not much out.

If you use a Random, you should ensure that you reuse a created instance. This is because if you create a random very shortly after another ( for Java 8: < 1ns which is a long time in computing) the Random will be created using the same seed it can happen that you will get the same "random" value.

private final static Random random = new Random();
public static final RegistrationNumber getInstance() {

   int numbers = random.nextInt(9000) + 1000;

   Character letter = (char)(random.nextInt(26) + 'a')
   ...  

Answer 4

In addition to what has already been mentioned, the string representation of the name is a derived property of the registration number and as such I believe that it should be computed internally in the constructor. This saves you one argument to the constructor and avoids code duplication if you have multiple factories.

Like this:

private RegistrationNumber(int number, char letter) {
  this.number = number; 
  this.letter = letter;
  this.strRep = String.format("%s%04d", letter, number);
}