Murder Your Darlings

July 07, 2010

Lately I’ve been working on connectivity with NASDAQ. The protocols involve parsing fixed-offset messages of various types. We’re not doing high frequency trading so we are optimizing for programmer efficiency — that is, the API I expose to the rest of the system should make sense, so I’m representing the different types of messages, trading conditions, exchange identifiers and so on as enums. The incoming message has a one-character code which I translate. I’ve seen programmers code this kind of thing using a big lookup table, but that leads to maintainability problems — when you add an enum value, did you remember to add it to the case statement? Did that case statement get copy and pasted elsewhere? The better solution is to embed that logic in the enum itself using a static map and a factory method.

enum SoupMessageType {
  LOGIN_REQUEST('L'),
  LOGIN_ACCEPT('A'),
  LOGIN_REJECT('J'),
  DATA('S'),
  LOGOUT_REQUEST('O');

  private char code;

  private SoupMessageType(char code) {
    this.code = code;
  }

  private static final Map<Character, SoupMessageType> map;
  static {
    map = new HashMap<Character, SoupMessageType>(values().length);
    for (SoupMessageType v : values()) {
      map.put(v.code(), v);
    }
  }

  public char code() {
    return code;
  }

  public SoupMessageType from(char code) {
    return map.get(code);
  }
}

This is a simple pattern, and I found myself copying it from another enum. Since copy and paste is bad, I started looking for how to turn this pattern into an abstraction. First, I’d move the static code block into the constructor for a map-like class:

public class CodedEnumer<K, E extends Enum<E> & CodedEnum<K>> {
  private Map<K, E> map;
  public CodedEnumer(Class<E> klass) {
    E[] enumConstants = klass.getEnumConstants();
    map = new HashMap<K, E>(enumConstants.length);
    for(E v : enumConstants) {
      map.put(v.code(), v);
    }
  }

  public static <K, V extends Enum<V> & CodedEnum<K>>
    CodedEnumer<K, V> create(Class<V> klass) {
    return new CodedEnumer<K, V>(klass);
  }

  public E get(K key) {
    return map.get(key);
  }
}

The enum needs to implement a CodedEnum interface with one method.

public interface CodedEnum<K> {
  public K code();
}

My first draft of this included another type parameter E for the enum, and a public E from(K key) method. But of course this method should be static, and declaring a static method in an interface would be meaningless (aside from the other detail of being a compiler error).

Now, rather than building maintaining the map itself, the enum needs to implement CodedEnum, create an instance of the CodedEnumer, and use that to implement the one-line static method.

enum SoupMessageTypeCoded implements CodedEnum<String, SoupMessageTypeCoded> {
  LOGIN_REQUEST('L'),
  LOGIN_ACCEPT('A'),
  LOGIN_REJECT('J'),
  DATA('S'),
  LOGOUT_REQUEST('O');

  private String code;

  private SoupMessageTypeCoded(String code) {
    this.code = code;
  }

  private static final CodedEnumer<String, SoupMessageTypeCoded>
    map = CodedEnumer.create(SoupMessageTypeCoded.class);

  @Override
  public String code() {
    return code;
  }

  public static SoupMessageTypeCoded from(String code) {
    return map.get(code);
  }
}

Pretty slick, huh? I was pretty pleased with myself when I actually found a use for intersection in a generic declaration. This is where the old writer’s advice of “murder your darlings” comes into play. It’s various attributed to Fitzgerald, Hemingway or others, but the meaning is that whenever you write a particularly clever turn of phrase, whatever makes you smile at how smart you are, get out the red pencil or delete key, and get rid of it.

Realistically, this code sucks. I’ve created two extra types with complicated generics to save two or three lines of code. Anyone who opens up the class in the future will have to open two more files to understand how it works and what it’s doing. So I hit delete and reverted to the version with those three horrible lines wastefully repeated in each and every enum I use this pattern in. I can only console myself that disk space is getting cheaper.