Secret Handshake Anti-Pattern

Question

I've just come across a pattern I've seen before, and wanted to get opinions on it. The code in question involves an interface like this:

public interface MyCrazyAnalyzer {
    public void setOptions(AnalyzerOptions options);
    public void setText(String text);
    public void initialize();
    public int getOccurances(String query);
}

And the expected usage is like this:

MyCrazyAnalyzer crazy = AnalyzerFactory.getAnalyzer();
crazy.setOptions(true);
crazy.initialize();
Map<String, Integer> results = new HashMap<String, Integer>();
for(String item : items) {
    crazy.setText(item);
    results.put(item, crazy.getOccurances);
}

There's reasons for some of this. The setText(...) and getOccurances(...) are there because there are multiple queries you might want to do after doing the same expensive analysis on the data, but this can be refactored to a result class.

Why I think this is so bad: the implementation is storing state in a way that isn't clearly indicated by the interface. I've also seen something similar involving an interface that required to call "prepareResult", then "getResult". Now, I can think of well designed code that employs some of these features. Hadoop Mapper interface extends JobConfigurable and Closeable, but I see a big difference because it's a framework that uses user code implementing those interfaces, versus a service that could have multiple implementations. I suppose anything related to including a "close" method that must be called is justified, since there isn't any other reasonable way to do it. In some cases, like JDBC, this is a consequence of a leaky abstraction, but in the two pieces of code I'm thinking of, it's pretty clearly a consequence of programmers hastily adding an interface to a spaghetti code class to clean it up.

My questions are:

1. Does everyone agree this is a poorly designed interface?
2. Is this a described anti-pattern?
3. Does this kind of initialization ever belong in an interface?
4. Does this only seem wrong to me because I have a preference for functional style and immutability?

If this is common enough to deserve a name, I suggest the "Secret Handshake" anti-pattern for an interface that forces you to call multiple methods in a particular order when the interface isn't inherently stateful (like a Collection).

Answer 1

1. I'd expect to see the AnalyzerFactory get passed the necessary params and do the construction itself; otherwise, what exactly is it doing?
2. Not sure if it does have a name, but it seems like it should :)
3. Yes, occassionally it's convenient (and the right level of abstraction) to have setters in your interface and expect classes to call them. I'd suggest that doing so requires extensive documentation of that fact.
4. Not really, no. A preference for immutability is certainly a good thing, and setter/bean based design can be the "right" choice sometimes too, but your given example is taking it too far.

Answer 2

I'm not sure whether it's a described anti-pattern but I totally agree this is a poorly designed interface. It leaves too much opportunity for error and violates at least one key principle: make your API hard to misuse.

Besides misuse, this API can also lead to hard-to-debug errors if multiple threads make use of the same instance.

Joshua Bloch actually has an excellent presentation (36m16s and 40m30s) on API design and he addresses this as one of the characteristics of a poorly designed API.

Answer 3

I can't see anything bad in here. setText() prepares the stage; after that, you have one or more calls to getOccurances(). Since setText() is so expensive, I can't think of any other way to do this.

getOccurances(text, query) would fix the "secret handshake" at a tremendous performance cost. You could try to cache text in getOccurances() and only update your internal caches when the text changes but that starts to look more and more like sacrifice to some OO principle. If a rule doesn't make sense, then don't apply it. Software developers have a brain for a reason.

Answer 4

Yes, it's an anti-pattern: Sequential coupling.

I'd refactor into Options - passed to the factory, and Results, returned from an analyseText() method.