I'd like to implement a Chain of Responsibility pattern, taking care of the 'broken link' problem as follows:
public abstract class Handler{
private Handler m_successor;
public void setSuccessor(Handler successor)
{
m_successor = successor;
}
protected abstract boolean handleRequestImpl(Request request);
public final void handleRequest(Request request)
{
boolean handledByThisNode = this.handleRequestImpl(request);
if (m_successor != null && !handledByThisNode)
{
m_successor.handleRequest(request);
}
}
}
Seems like a common enough approach. But how is this testable with a protected abstract method? Ways to handle this seem to be:
- Implement a test-only subclass of
Handler
that implements the abstract method. This seems bad for test maintenance. - Change the visibility of the abstract method to public, but I shouldn't need to change the SUT to accomodate the test.
- Regard the abstract class as sufficiently simple to not require unit tests. Mmmm.
- Implement unit tests for the
handleRequest
method on one or more concrete subclasses. But this doesn't seem like a sensible way to organize tests. - Is there someway to use a mock object? I've tried Mockito, but can't seem to get around the protected visibility.
I've read[1] that this sort of testing problem implies that the design is wrong, and suggest using composition rather than inheritence. I'm trying this now, but it seems strange that the recommended implementation of this pattern has this problem, yet I can't find any advice about unit testing it.
UPDATED: I've replaced the abstract class with dependency inversion as shown, and this is now easily easily testable using Mockito. It still looks like Chain of Responsibility... am I missing something?
// Implement a concrete class instead
public class ChainLink {
// Successor as before, but with new class type
private ChainLink m_successor;
// New type, RequestHandler
private RequestHandler m_handler;
// Constructor, with RequestHandler injected
public ChainLink(RequestHandler m_handler) {
this.m_handler = m_handler;
}
// Setter as before, but with new class type
public void setSuccessor(ChainLink successor) {
m_successor = successor;
}
public final void handleRequest(Request request) {
boolean handledByThisNode = m_handler.handleRequest(request);
if (m_successor != null && !handledByThisNode) {
m_successor.handleRequest(request);
}
}
}
If you use PowerMock + Mockito, you can write a test similar to this:
Which will verify that your second handler's method is called when the first one returns false and that it is not called when it returns true.
Another option is to follow the well-known rule of "favor composition over inheritance" and change your class to something like this:
Then you can mock it in this way (or use pretty much any mock framework since you don't have protected methods):
In this solution, you can also make Handler inherit after Callable and then you are able to wrap it over any other callable which may have a successor and use the handler instead of the original callable; it is much more flexible.
I would go for option 1. That fake sub class is simple enough and you can place it in your tets class. There is nothing wrong with test-only sub classes.