Wednesday, April 6, 2011

Avoiding casts when translating public APIs to internal glue code

So, I have this public API that my application exposes, allowing customers to write plug-ins. For the sake of this example, let's say it's a fairly simple key-value pair system, something like:

public interface Key {
  // marker interface, guaranteed unique in some scope
}

public interface KVPService {
  Set<Key> getKeys();
  Object getValue(Key k); // must be a key provided by getKeys() or it blows up
}

Now let's say that internally, the Key has some attributes that shouldn't be exposed -- say a database ID. What I'm currently doing is something like this:

/** Internal impl of external Key */    
class InternalKey implements Key {
  int getDatabaseId() {
    // do something...
  }
}

/** Internal impl of external KVPService */    
class InternalKVPService implements KVPService {
  // ...

  public Object getValue(Key k) {
    InternalKey ik = (InternalKey) k;
    return getValueFromDBOrWherever(ik.getDatabaseId());
  }

  // ...
}

This seems less than ideal. Is there some way I can rearrange the responsibilities to avoid the cast and still maintain the inside/outside encapsulation?

Note that in the real world it's a fair bit more complicated than this; the Key equivalents have a fair bit of metadata attached and there's a number of things someone might want to do with one other than get a simple value, so just exposing, say, Map.Entry-like objects instead won't necessarily solve the problem.

The only thing I've been able to come up with is to separate the internal and external keys entirely, and keep around a Map<Key, InternalKey>. But in that case I'd have to either copy the metadata, which violates DRY, or have the external Key delegate to InternalKey, in which case the Map violates DRY.

Can anyone think of something more clever?

From stackoverflow
  • I don't think so. But why does the typecast worries you? The potential ClassCastException won't happen in practice (assuming that I understand your use-case), the cast will only take 5 or so machine instructions, and it is hidden from callers of your KPService API.

    David Moles : I think mainly I don't like that I can't prove at compile-time that a `Key` is an `InternalKey`. But given that I'm getting `Key`s from the external caller, maybe it's inherently unreasonable to want that.
    Robin : If you want the compile time checking, you are going to have to either use classes or generics on your interfaces. Assuming you can change your public API.
    Stephen C : @Robin: even with generics, the JVM often has to do a type-cast underneath the covers.
  • One approach I've seen is to expose a common interface for all objects (in this case keys) and provide a base implementation that simply throws an UnsupportedOperationException (or do nothing) for each method. Then sub-class implementations subsequently override method(s) to provide functionality. Granted it's not very OO-like but you'll find some examples in the JDK API (e.g. Iterator's remove() method).

    Another option: You could use the visitor pattern to have each object perform functionality without downcasting; e.g.

    public interface KeyVisitor {
      void visitInternalKey(InternalKey k);
      void visitFooBarKey(FooBarKey k);
    }
    
    public interface Key {
      void visit(KeyVisitor kv);
    }
    
    public class InternalKey implements Key {
      public void visit(KeyVisitor kv) {
        kv.visitInternalKey(this);
      }
    }
    

    The disadvantage here is that you would have to expose the methods on InternalKey (at least via an interface) to allow your visitor implementations to call them. However, you could still keep the implementation detail at the package level.

    David Moles : There's enough different things I need to do to `Key` that I'm not sure a straight Visitor pattern will work, but that at least puts the responsibilities in the right place. Good thinking.
    Adamski : Thanks. Only thing to bear in mind: If you over-use Visitor it can make code fairly unreadable - Sometimes a switch / if-then statement is actually easier to maintain (and probably faster).

0 comments:

Post a Comment