Alex is a Software Engineer working on Android development tools, especially Android Studio, at Google. His interests include Java, API design, OOP, IDEs and testing. Alex spends some of his spare time working on Open Source, blogging, writing technical articles, and speaking at international conferences. The opinions expressed here represent his own and not those of his employer. Alex is a DZone MVB and is not an employee of DZone and has posted 49 posts at DZone. You can read more from them at their website. View Full User Profile

An (Overlooked?) Use Case for the Strategy Pattern

01.29.2010
| 9585 views |
  • submit to reddit

Composition over inheritance. It seems that we all agree and understand this valuable principle. I’ve been reading a good amount of code lately from some open source projects (including mine,) and I think we are still missing at least one use case where to apply it.

This is what I’ve seen: class A has method x. Just after method x is done doing its thing and before it exits, it calls a (sometimes empty) protected method y meant to be overridden to handle some result or side effect of x (is there a name for this anti-pattern?)

Here is a concrete (and oversimplified) example, taken from FEST:

/**
* Understands a <code>{@link SecurityManager}</code> that does not allow an application
* under test to terminate the current JVM. Adapted from Abbot's own
* {@code SecurityManager}.
*/
public class NoExitSecurityManager extends SecurityManager {
@Override public final void checkExit(int status) {
if (!exitInvoked()) return;
handleExitRequest(status);
throw new ExitException(concat(
"Application tried to terminate current JVM with status ", status));
}

/**
* Implement this method to do any context-specific cleanup. This hook is provided since
* it may not always be possible to catch the <code>{@link ExitException}</code>
* explicitly (like when it's caught by someone else, or thrown from the event dispatch
* thread).
* @param status the status the exit status.
*/
protected void handleExitRequest(int status) {}
}

At first glance this code looks OK. Subclasses can do whatever they want when “exit” is called, and since the method checkExit is final, there is no way that subclasses can accidentally (or intentionally) change the intended behavior of the super class.

There are, IMHO, some issues with this approach:

Let's take a look at a simple (silly?) subclass of NoExitSecurityManager that writes a message to a file when exit is called:

public class WriteToFileNoExitSecurityManager extends NoExitSecurityManager {
// The class FileWriter only exists for the purposes of this example
private final FileWriter fileWriter = new FileWriter("${temp}/log.txt");

@Override protected void handleExitRequest(int status) {
fileWriter.write(concat("Exit called with status ", status));
}
}

This subclass is actually doing too much: it is preventing an application from exiting (inherited behavior) and it is also handling the “exit request.”

WriteToFileNoExitSecurityManager is not really a NoExitSecurityManager, but an “exit request handler.” Inheritance in this case is, IMHO, unnecessary: the implemented functionality (not the inherited one) in WriteToFileNoExitSecurityManager is self-contained, and it does not depend on the internal state (or identity) of the superclass. In addition, since the method checkExit is final, there can only be one implementation. On the other hand, there can be different implementations of handleExitRequest, each of them forced to be a subclass of NoExitSecurityManager.

Testing is also more complicated in this scenario. To test NoExitSecurityManager, we would need to subclass it (more unnecessary inheritance) and set some flag in handleExitRequest to verify it is called (we could also create a mock, but at the end it’s all the same):

public class NoExitSecurityManagerTest {
private TestNoExitSecurityManager securityManager;

@Before public void setUp() {
securityManager = new TestNoExitSecurityManager();
}

@Test public void should_prevent_application_from_exiting_and_handle_exit_request() {
// test that an application trying to exit is effectively stopped
...
// here we verify that the call to 'handleExitRequest' was made
assertThat(securityManager.handleExitRequestCalled).isTrue();
}

private static class TestNoExitSecurityManager extends NoExitSecurityManager {
boolean handleExitRequestCalled;

@Override protected void handleExitRequest(int status) {
handleExitRequestCalled = true;
}
}
}

Enter the Strategy pattern

We can clean up our example by using the strategy pattern.

First, we introduce the interface ExitRequestHandler. It only has one responsibility: to handle “exit requests.”

public interface ExitRequestHandler {
void handleExitRequest(int status);
}

Then, we can replace the call to the method handleExitRequest with a call to an instance of ExitRequestHandler:

/**
* Understands a <code>{@link SecurityManager}</code> that does not allow an application
* under test to terminate the current JVM. Adapted from Abbot's own
* {@code SecurityManager}.
*/
public final class NoExitSecurityManager extends SecurityManager {
private final ExitRequestHandler exitRequestHandler;

public NoExitSecurityManager(ExitRequestHandler exitRequestHandler) {
this.exitRequestHandler = exitRequestHandler;
}

@Override public void checkExit(int status) {
if (!exitInvoked()) return;
exitRequestHandler.handleExitRequest(status);
throw new ExitException(concat(
"Application tried to terminate current JVM with status ", status));
}
}

Finally, we can rewrite WriteToFileNoExitSecurityManager as a WriteToFileExitRequestHandler:

public class WriteToFileExitRequestHandler implements ExitRequestHandler {
// The class FileWriter only exists for the purposes of this example
private final FileWriter fileWriter = new FileWriter("${temp}/log.txt");

@Override public void handleExitRequest(int status) {
fileWriter.write(concat("Exit called with status ", status));
}
}

It looks like I just moved code around and actually created even more code! True, we have more code, but better-quality code:

  • each class has a single, well-defined responsibility
  • there is no tight coupling between the class using the strategy interface and a specific strategy implementation
  • implementing a small interface is easier than extending a class (no need to worry about the state of the superclass in a particular context)
  • testing is simpler

Here is the updated test:

public class NoExitSecurityManagerTest {
private ExitSecurityManager securityManager;
private TestExitRequestHandler requestHandler;

@Before public void setUp() {
requestHandler = new ExitRequestHandler();
securityManager = new ExitSecurityManager(requestHandler);
}

@Test public void should_prevent_application_from_exiting_and_handle_exit_request() {
// test that an application trying to exit is effectively stopped
...
// here we verify that the call to 'handleExitRequest' was made
assertThat(requestHandler.handleExitRequestCalled).isTrue();
}

private static class TestExitRequestHandler implements ExitRequestHandler {
boolean handleExitRequestCalled;

@Override public void handleExitRequest(int status) {
handleExitRequestCalled = true;
}
}
}

I had a hard time writing this blog post because the points I just made seemed to be too obvious and well-understood by now. But, by reading code (both mine and written by others) I got the impression that although we understand the “composition over inheritance” concept, it is still not easy to apply it in practice.

Am I being too picky? I hope not (although I admit this issue has been annoying me for some time, I finally had the chance to complain.) IHMO, every detail counts when writing clean code :)

Feedback is always welcome!

From http://alexruiz.developerblogs.com

Published at DZone with permission of Alex Ruiz, author and DZone MVB.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Tags:

Comments

Hasani Jess replied on Fri, 2010/01/29 - 5:36am

A common approach to this problem that I like is to implement Listeners (is there a listener patter?!?).

Where effectively your handler would be a listener that responds to onExitRequest. A small change to your program to contain a collection of listeners would then mean that you could process a number of listeners, e.g write to file, send to jms queue, send email etc.

The only difference with listeners is that normally they are not mandatory, which it appears your example requires. 

Andrew McVeigh replied on Fri, 2010/01/29 - 6:13am in response to: Hasani Jess

> is there a listener pattern?!?

 yes -- it's called the observer pattern: http://en.wikipedia.org/wiki/Observer_pattern

Hasani Jess replied on Fri, 2010/01/29 - 11:55am in response to: Andrew McVeigh

doh!

Alexander Lee replied on Fri, 2010/01/29 - 12:11pm

No, its a pretty well known implementation of the Strategy pattern.  I Java, take the Comparable interface (though a example of impl) and the ability to pass a Comparator to a Collections.sort(...)/Arrays.sort(...).  The former is an example of inheritance, i.e. you implement the Comparable interace and method compareTo(...) in your class.  The second is an example of the Strategy pattern where you don't need a compareTo(...) method in your class as the compare logic is in the Strategy/Comparator which you have implemented.

So I guess the answer here is that both ways are valid.  It depends on the class you are implementing and the function.  In the example above, it makes sense to use a Comparator and the Strategy pattern as a sort is not intrinsic to the class being implemented, and you may wish to sort a number of different ways by swapping in different Strategies/Comparators, which a concrete implementation of compareTo(...) would prevent.  That said, if we were to talk about the equals(...) method, this is not a good candidate (in general) for a Strategy pattern as the implementation of equals(...) is intrinsic to the implementation of the class.

In theory, you could use the Strategy pattern and composition to replace every method implementation in a class you are writing, question is when you should.  If the implementation/logic is intrinsic and tighly bound to the class then it makes sense to  make that logic part of the class.  If it something external to the meaning of the class (such as a sort algorithm) then it makes sense as a strategy.

In the example you specify above, it is border line, especially as the method to implement is not abstract, therefore optional, which would imply that the functionality is not intrinsic, similiar to the finalise() method in Java, which could be implemented as a Strategy.

Alexander Lee replied on Fri, 2010/01/29 - 12:22pm

Oh, and the full best practice rule is "prefer composition over inheritance".

The word "prefer" is there for a reason, because in many cases the correct approach is to use inheritence.  The problem with inheritence is that it is in many cases over used, or used incorrectly.

Mario Schwede replied on Fri, 2010/01/29 - 6:53pm

Yes! You can implement TemplateMethod without inheritence. Just choose strategy for the point of variation.

Joseph Bradington replied on Sat, 2010/01/30 - 7:45am

Nice article. These kinds of articles would be even more useful if they discussed criteria for when to use one pattern or the other (TemplateMethod vs. Strategy in this case). For example, if the overridden method will may need access to protected state in the defining class then a TemplateMethod pattern would be more straightforward than using a strategy (and passing some type of context object that represents the state needed by the strategy). It arguable, but using a TemplateMethod with non-abstract methods implies a Strategy pattern is more appropriate. If empty methods are used with the TemplateMethod is shows that the methods are not required for the templated algorithm being implemented by the class. I'm sure there are other considerations, but these are just a few samples.

 

Mark Bednarczyk replied on Sat, 2010/01/30 - 12:33pm in response to: Alexander Lee

The problem with inheritence is that it is in many cases over used, or used incorrectly.
I agree with you 100%. Just look at swing and AWT, how much stuff is in the components. Way over used.

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.