Question

I was just looking at the Java Hamcrest code on GitHub, and noticed they employed a strategy that seemed unintuitive and awkward, but it got me wondering if I'm missing something.

I noticed in the HamCrest API that there is an interface Matcher and an abstract class BaseMatcher. The Matcher interface declares this method, with this javadoc:

    /**
     * This method simply acts a friendly reminder not to implement Matcher directly and
     * instead extend BaseMatcher. It's easy to ignore JavaDoc, but a bit harder to ignore
     * compile errors .
     *
     * @see Matcher for reasons why.
     * @see BaseMatcher
     * @deprecated to make
     */
    @Deprecated
    void _dont_implement_Matcher___instead_extend_BaseMatcher_();

Then in BaseMatcher, this method is implemented as follows:

    /**
     * @see Matcher#_dont_implement_Matcher___instead_extend_BaseMatcher_()
     */
    @Override
    @Deprecated
    public final void _dont_implement_Matcher___instead_extend_BaseMatcher_() {
        // See Matcher interface for an explanation of this method.
    }

Admittedly, this is both effective and cute (and incredibly awkward). But if the intention is for every class that implements Matcher to also extend BaseMatcher, why use an interface at all? Why not just make Matcher an abstract class in the first place and have all other matchers extend it? Is there some advantage to doing it the way Hamcrest has done it? Or is this a great example of bad practice?

EDIT

Some good answers, but in search of more detail I'm offering a bounty. I think that the issue of backwards / binary compatibility is the best answer. However, I'd like to see the issue of compatibility elaborated on more, ideally with some code examples (preferably in Java). Also, is there a nuance between "backwards" compatibility and "binary" compatibility?

FURTHER EDIT

January 7, 2014 -- pigroxalot provided an answer below, linking to this comment on Reddit by the authors of HamCrest. I encourage everyone to read it, and if you find it informative, upvote pigroxalot's answer.

EVEN FURTHER EDIT

December 12, 2017 -- pigroxalot's answer was removed somehow, not sure how that happened. It's too bad... that simple link was very informative.

Was it helpful?

Solution

The git log has this entry, from December 2006 (about 9 months after the initial checkin):

Added abstract BaseMatcher class that all Matchers should extend. This allows for future API compatability [sic] as the Matcher interface evolves.

I haven't tried to figure out the details. But maintaining compatibility and continuity as a system evolves is a difficult problem. It does mean that sometimes you end up with a design that you would never, ever, ever have created if you had designed the whole thing from scratch.

OTHER TIPS

But if the intention is for every class that implements Matcher to also extend BaseMatcher, why use an interface at all?

It's not exactly the intent. Abstract base classes and interfaces provide entirely different 'contracts' from an OOP perspective.

An interface is a communication contract. An interface is implemented by a class to signify to the world that it adheres to certain communication standards, and will give a specific type of result in response to a specific call with specific parameters.

An abstract base class is an implementation contract. An abstract base classes is inherited by a class to provide functionality that is required by the base class but left for the implementer to provide.

In this case, both overlap, but this is merely a matter of convenience - the interface is what you need to implement, and the abstract class is there to make implementing the interface easier - there is no requirement whatsoever to use that base class to be able to offer the interface, it's just there to make it less work to do so. You are in no way limited in extending the base class for your own ends, not caring about the interface contract, or in implementing a custom class implementing the same interface.

The given practice is actually rather common in old-school COM/OLE code, and other frameworks facilitating inter-process communications (IPC), where it becomes fundamental to separate implementation from interface - which is exactly what is done here.

I think what happened is that initially a Matcher API was created in the form of an interface.
Then while implementing the interface in various ways a common code base was discovered which was then refactored out into the BaseMatcher class.

So my guess is that the Matcher interface was retained as it is part of the initial API and the descriptive method was then added as a reminder.

Having searched through the code, I found that the interface could easily by done away with as it is ONLY implemented by BaseMatcher and in 2 test units which could easily be changed to use BaseMatcher.

So to answer your question - in this particular case there is no advantage to doing it this way, besides not breaking other peoples implementations of Matcher.

As to bad practice ? In my opinion it is clear and effective - so no I don't think so, just a little odd :-)

Hamcrest provides matching, and matching only. It is a tiny niche market but they appear to be doing it well. Implementations of this Matcher interface are littered across a couple unit testing libraries, take for example Mockito's ArgumentMatcher and across a silly great amount of tiny anonymous copy-paste implementations in unit tests.

They want to be able to extend the Matcher with a new method without breaking all of those existing implementing classes. They would be hell to upgrade. Just imagine suddenly having all your unittest classes showing angry red compile errors. The anger and annoyance would kill hamcrest's niche market in one quick swoop. See http://code.google.com/p/hamcrest/issues/detail?id=83 for a small taste of that. Also, a breaking change in hamcrest would divide all versions of libraries that use Hamcrest into before and after the change and make them mutually exclusive. Again, a hellish scenario. So, to keep some freedom, they need Matcher to be an abstract base class.

But they are also in the mocking business, and interfaces are way easier to mock than base classes. When the Mockito folks unit test Mockito, they should be able to mock the matcher. So they also need that abstract base class to have a Matcher interface.

I think they have seriously considered the options and found this to be the least bad alternative.

There is an interesting discussion about it here. To quote nat_pryce:

Hi. I wrote the original version of Hamcrest, although Joe Walnes added this wierd method to the base class.

The reason is because of a peculiarity of the Java language. As a commenter below said, defining Matcher as a base class would make it easier to extend the library without breaking clients. Adding a method to an interface stops any implementing classes in client code from compiling, but new concrete methods can be added to an abstract base class without breaking subclasses.

However, there are features of Java that only work with interfaces, in particular java.lang.reflect.Proxy.

Therefore, we defined the Matcher interface so that people could write dynamic implementations of Matcher. And we provided the base class for people to extend in their own code so that their code would not break as we added more methods to the interface.

We have since added the describeMismatch method to the Matcher interface and client code inherited a default implementation without breaking. We also provided additional base classes that make it easier to implement describeMismatch without duplicating logic.

So, this is an example of why you can't blindly follow some generic "best practice" when it comes to design. You have to understand the tools you're using and make engineering trade-offs within that context.

EDIT: separating the interface from the base class also helps one cope with the fragile base class problem:

If you add methods to an interface that is implemented by an abstract base class, you may end up with duplicated logic either in the base class or in subclasses when they are changed to implement the new method. You cannot change the base class to remove that duplicated logic if doing so changes the API provided to subclasses, because that will break all subclasses -- not a big problem if the interface and implementations are all in the same codebase but bad news if you're a library author.

If the interface is separate from the abstract base class -- that is, if you distinguish between users of the type and implementers of the type -- when you add methods to the interface you can add a default implementation to the base class that will not break existing subclasses and introduce a new base class that provides a better partial implementation for new subclasses. When someone comes to change existing subclasses to implement the method in a better way, then can choose to use the new base class to reduce duplicated logic if it makes sense to do so.

If the interface and base class are the same type (as some have suggested in this thread), and you then want to introduce multiple base classes in this way, you're stuck. You can't introduce a new supertype to act as the interface, because that will break client code. You can't move the partial implementation down the type hierarchy into a new abstract base class, because that will break existing subclasses.

This applies as much to traits as Java-style interfaces and classes or C++ multiple inheritance.

Java8 now allows new methods to be added to an interface if they contains default implementations.

interface Match<T>

    default void newMethod(){ impl... }

this is a great tool that gives us a lot of freedom in interface design and evolution.

However, what if you really really want to add an abstract method that has no default implementation?

I think you should just go ahead and add the method. It'll break some existing codes; and they will have to be fixed. Not really a big deal. It probably beats other workarounds that preserve binary compatibility at the cost of screwing up the whole design.

But if the intention is for every class that implements Matcher to also extend BaseMatcher, why use an interface at all? Why not just make Matcher an abstract class in the first place and have all other matchers extend it?

By separating interface and implementation (abstract class is still an implementation) you comply with Dependency Inversion Principle. Do not confuse with dependency injection, nothing in common. You might notice that, in Hamcrest interface is kept in hamcrest-api package, while abstract class is in hamcrest-core. This provides low coupling, because implementation depends only on interfaces but not on other implementation. A good book on this topic is: Interface Oriented Design: With Patterns.

Is there some advantage to doing it the way Hamcrest has done it? Or is this a great example of bad practice?

The solution in this example looks ugly. I think comment is enough. Making such stub methods is redundant. I wouldn't follow this approach.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top