Good design for class with similar constructors

Question

I was reading this question and thought that good points were made, but most of the solutions involved renaming one of the methods. I am refactoring some poorly written code and I've run into this situation:

public class Entity {

  public Entity(String uniqueIdentifier, boolean isSerialNumber) {
    if (isSerialNumber) {
      this.serialNumber = uniqueIdentifier;
      //Lookup other data
    } else {
      this.primaryKey = uniqueIdentifier;
      // Lookup other data with different query
    }
  }

}

The obvious design flaw is that someone needed two different ways to create the object, but couldn't overload the constructor since both identifiers were of the same type (String). Thus they added a flag to differentiate.

So, my question is this: when this situation arises, what are good designs for differentiating between these two ways of instantiating an object?

My First Thoughts

1. You could create two different static methods to create your object. The method names could be different. This is weak because static methods don't get inherited.
2. You could create different objects to force the types to be different (i.e., make a PrimaryKey class and a SerialNumber class). I like this because it seems to be a better design, but it also is a pain to refactor if serialNumber is a String everywhere else.

Answer 1

One thing you can do is subclass Entity and define specific constructors for each. I would wage a bet that there's other code that could be moved into the subclasses from the parent one. If not right now, then probably in the future. In either case, you still have all the common functionality in Entity.

public class SerialEntity extends Entity{
    public SerialEntity(String serialNumber){
        this.serialNumber = serialNumber;
    }
}

public class PKEntity extends Entity{
    public PKEntity(String primaryKey){
        this.primaryKey= primaryKey;
    }
}

Perhaps at the same time make Entity abstract? But that's slightly out of scope here.

However, if the only thing different about Entity is the identifier, then create an EntityIdentifier interface with implementations for each type of identifier. This way Entity only needs one constructor with a single argument.

public class SerialIdentifier implements EntityIdentifier{
    private final String serialNumber;

    public SerialIdentifier(String serialNumber){
        this.serialNumber = serialNumber;
    }

    @Override
    public String getKey(){
        return serialNumber;
    }
}

public class Entity{
    public Entity(EntityIdentifier id){
        this.id = id;
    }
}

(This assumes you only ever need serialNumber xor primaryKey, though.)

I think the identifier approach is complementary to creating specific types for the values themselves. That is, if your application really needs a SeraialNumber type, but doesn't need a "pimary key" type, then you can easily refactor the SerialIdentifier above to work with the new type. This also decouples SerialNumber from being an identifier type, if such decoupling is necessary. The beauty of this is that you wouldn't have to worry about the rest of the Entity class because it already operates against the EntityIdentifier interface and is independent of what the "id" actually is.

Answer 2

Personally, my solution to this would be to factor the constructor to take no parameters and use setter injection. After all, if half the time serial number can be uninitialized and half the time primary key can be uninitialized, it's obvious that neither one is essential to the functioning of the class. Since that's the case, forget the awkward primitive gymnastics and leave it to clients to specify what data they have.

This may reveal a requirement that one or the other be set, but that would seem to lead to factoring two classes out of the target class rather than represent some kind of C-style union structure.

Answer 3

I like the second option you've given. It's type safe, and to me, more intuitive.

I'm not sure if the code here is C# or Java, but if it's C#, you could take your second option and add an implicit conversion to string to help with the other places where serialNumber might be used as a string.

public class SerialNumber
{
    private string serialNumber = null;

    public SerialNumber(string serialNumber)
    {
        this.serialNumber = serialNumber;
    }

    public static implicit operator string(SerialNumber sn)
    {
        return sn.serialNumber;
    }
}

Of course, implicit operators have been frowned upon in the past, so you'd have to weigh the pros and cons.

Answer 4

Number 2 is easily the best solution. It allows you to do validation and provides an actual distinction in the type system for distinct types.