tech blog

During a recent refactoring of the LimeWire code base we pulled the search trie based keyword index out of the FileManagerImpl class and put it into its own class SharedFilesKeywordIndexImpl.

The new SharedFilesKeywordIndexImpl listens for file manager add/remove events and updates its search trie accordingly. The file manager is no longer coupled to the index. The implementation is abstracted away behind the SharedFilesKeywordIndex interface which is used in the components that need it:

public interface SharedFilesKeywordIndex extends FileEventListener {

    public abstract Response[] query(QueryRequest request);

}

If you wanted to implement a Gnutella client with a different behavior that allowed other clients to still browse your client's shared files but not to search for them, you might be tempted to implement your own NullSharedFilesKeywordIndex that just didn't return any responses. And indeed, what a nice workaround this would be, at least better than before where you would have had to subclass FileManagerImpl.

So what's the problem here?

Looking at the LimeWire code to find out where SharedFilesKeywordIndex is used, we find out that we also just disabled the clients functionality that allows the user to search through their own shared files from the user interface. The class is used in this context too.

The problem is that the SharedFilesKeywordIndex doesn't represent the actual outward behavior of the program but constitutes internal behavior that is used in different ways to produce outward behavior. Thus, the keyword index must be agnostic of its callers and can't make decisions as to whether deliver responses or not.

So where's the right place to change external behavior?

If you imagine all the objects in a program as part of a graph that visualizes their respective dependencies on each other, you should only replace the leaf nodes in that graph, if you want to make sure you only change the outward behavior in one place. As soon as you replace an inner node object, you run the risk of changing the behavior of more than one leaf node.

Back to our example, we find out that the class StandardMessageRouter contains the external behavior that we want to change: It holds the SharedFilesKeywordIndex and queries it when it receives an incoming query request. Ideally, this logic should be moved out of the message router class and just be a small message listener implementation. This would be the new leaf node that depends on the keyword index node.

Creating small classes like this with a clear separation of concerns in mind, also takes away the fear of not reusing enough code. These small classes often become trivial to implement and reimplement, and just tie two other components together.

The drawback of this approach is the myriad of classes and interfaces that need to be introduced and the extra burden that is put on wiring classes together and the desire to make that wiring code, which will be substantial in size, reusable too.

I think it is still a good idea, to identify the roles that classes play inside the program graph and in the case of singletons to make sure they are not swapped out for something else if they inner nodes of the dependency graph.

It might even be interesting to come up with a nomenclature for object roles (INTERNAL, EXTERNAL), and annotate classes with them.

P.S.: Looking at the code snippet of the SharedFilesKeywordIndex above, you'll notice a problem. It extends the file manager listener interface and thus gives away a detail of the default implementation that happens to be listen to file manager events.