One way to remove duplication is using inheritance. However, in many cases composition is better and just as easy to implement.
Consider the following code:
public class UpperCaseEmphasiser {
public String emphasise(String something) {
StringBuffer result = new StringBuffer();
StringTokenizer tokens = new StringTokenizer(something, " ", true);
while(tokens.hasMoreTokens()){
String token = tokens.nextToken();
if(!token.equals(" ")){
token = token.toUpperCase();
}
result.append(token);
}
return result.toString();
}
}
with the following tests:
import junit.framework.TestCase;
public class UpperCaseEmphasiserTest extends TestCase {
private UpperCaseEmphasiser emphasiser = new UpperCaseEmphasiser();
public void testThatEmphasiserDoesNotPutAnyEmphasisOnEmptyString() {
assertEquals("", emphasiser.emphasise(""));
}
public void testThatEmphasiserPutsEmphasisOnOneWord() {
assertEquals("HELLO", emphasiser.emphasise("hello"));
}
public void testThatEmphasiserPutsEmphasisOnMultipleWords() {
assertEquals("HELLO WORLD", emphasiser.emphasise("hello world"));
}
}
and another class called "AsteriskEmphasiser", which is exactly the same except having "token = ""token"";" instead of "token = token.toUpperCase();" and the tests for it being slightly different as appropriate.
There is a lot of duplication between "UpperCaseEmphasiser" and "AsteriskEmphasiser" and also between their tests. One way this duplication can be removed is using inheritance, creating an abstract class for the duplicated code and having the concrete subclasses containing only the code that is different:
public abstract class Emphasiser {
public String emphasise(String something) {
StringBuffer result = new StringBuffer();
StringTokenizer tokens = new StringTokenizer(something, " ", true);
while(tokens.hasMoreTokens()){
String token = tokens.nextToken();
if(!token.equals(" ")){
token = emphasiseWord(token);
}
result.append(token);
}
return result.toString();
}
abstract String emphasiseWord(String token);
}
and then UpperCaseEmphasiser becomes:
public class UpperCaseEmphasiser extends Emphasiser {
String emphasiseWord(String token) {
return token.toUpperCase();
}
}
and "AsteriskEmphasiser" is modified similarly.
Using inheritance has eliminated the duplication between the classes, but not between the tests. Sometimes, people will use inheritance in the tests to eliminate that duplication too, however, I suggest that using composition instead is better, because it separates the code into the functionality that is shared and the functionality that is different, in a way in which the functionality that is different can be called independently of the code that is shared. In this example, Emphasiser can be modified as:
public class Emphasiser {
private WordEmphasiser wordEmphasiser;
public Emphasiser(WordEmphasiser wordEmphasiser){
this.wordEmphasiser = wordEmphasiser;
}
private String emphasiseWord(String token){
return wordEmphasiser.emphasiseWord(token);
}
public String emphasise(String something) {
... unchanged ...
and UpperCaseEmphasiser becomes:
public class UpperCaseEmphasiser implements WordEmphasiser {
public String emphasiseWord(String token) {
return token.toUpperCase();
}
}
(and WordEmphasiser is:
public interface WordEmphasiser {
String emphasiseWord(String token);
}
)
and the tests can be kept the same by doing:
public class UpperCaseEmphasiserTest extends TestCase {
private Emphasiser emphasiser = new Emphasiser(new UpperCaseEmphasiser());
... unchanged ...
The code as shown so far isn't a big change from using inheritance, but does have a big effect. With this arrangement, instead of having the tests duplicated (or using inheritance in the tests to remove the duplication) the tests can now be split into two parts. One test for the class with the code that is common, and separate tests for the code that is different. The tests for the common code now becomes:
public class EmphasiserTest extends TestCase {
private MockWordEmphasiser aMockWordEmphasiser = new MockWordEmphasiser();
class MockWordEmphasiser implements WordEmphasiser {
public Stack expectedWords = new Stack();
public String emphasiseWord(String word) {
String expectedWord = (String)expectedWords.pop();
assertEquals(expectedWord, word);
return word;
}
}
private Emphasiser emphasiser = new Emphasiser(aMockWordEmphasiser);
public void testThatEmphasiserDoesNotPutAnyEmphasisOnEmptyString() {
Object ensureEmphasiseWordIsNotCalled = new Object();
aMockWordEmphasiser.expectedWords.add(ensureEmphasiseWordIsNotCalled);
assertEquals("", emphasiser.emphasise(""));
}
public void testThatEmphasiserPutsEmphasisOnOneWord() {
aMockWordEmphasiser.expectedWords.add("hello");
assertEquals("hello", emphasiser.emphasise("hello"));
}
public void testThatEmphasiserPutsEmphasisOnMultipleWords() {
aMockWordEmphasiser.expectedWords.add(0,"hello");
aMockWordEmphasiser.expectedWords.add(0,"world");
assertEquals("hello world", emphasiser.emphasise("hello world"));
}
}
which is completely independent of the implementations of any "WordEmphasiser". There are many other ways to implement "Mock" objects, but that is beyond the scope of this article. Here I've presented a simple implementation which I hope is clear as it does not need any knowledge or reference to anything outside of the code presented.
Note that now the test is testing that the "WordEmphasiser" is called with the correct arguments and the "Emphasiser" returns the words converted using whatever the "WordEmphasiser" returns (in this case, making no change to the words).
The tests for the "UpperCaseEmphasiserTest" are now very simple:
public class UpperCaseEmphasiserTest extends TestCase {
private WordEmphasiser emphasiser = new UpperCaseEmphasiser();
public void testThatEmphasiserPutsEmphasisOnOneWord() {
assertEquals("HELLO", emphasiser.emphasiseWord("hello"));
}
}
and similarly for "AsteriskEmphasiserTest". Note that now one test is sufficient, because the WordEmphasiser is only called for one word at a time - it isn't called for spaces or multiple words, because that is taken care of by the "Emphasiser". This is an example of a "separation of concerns"; i.e. the splitting of a string into words has been separated from the application of emphasis to each word. There are other benefits in addition to the simplification of the tests, and the removal of duplication. Using composition also allows more flexibility in combining these classes together. For example, there could be an "Emphasiser" which puts emphasis on every other word, which could reuse the "WordEmphasisers" independently of thier use by the standard "Emphasiser".
Whenever you see inheritance, particularly tests using inheritance, consider whether converting to composition would allow better separatation of concerns, making the tests clearer and the code cleaner.
One of the main activities of the the Design Patterns Revisited workshop, lead by Brian Foote, James Noble, Kyle Brown, and Dirk Riehle, was voting about which patterns from the GOF book (now 10 years old) should be included, removed or revised for a (at this point theoretical) second edition. I won't repeat Martin Fowler's write-up, but rather I'm writing up my reasons for wanting to remove some of the patterns. I should make it clear - I'd like a GOF style book in two volumes, one without these patterns (that I could happily recommend to anyone) and a second volume with these, and other, patterns for people with sufficient experience not to misuse them.
The intent of Singleton is to "Ensure a class only has one instance, and provide a global point of access to it". Martin Fowler pointed out that it is most often used for the "global point of access" rather than the "only has one instance" part. There are at least two problems with having global access to things. One is that it hides dependencies so that it is more difficult to understand how some piece of code works and how different pieces of code affect each other. Another is that it makes it more difficult to substitute the global thing with something else for testing purposes. Rather than have hidden dependencies using globals, the dependency injection approach makes dependencies explicit and easy to substitute for testing. If you really want a "global point of access" type thing, have a look at the Registry pattern.
The problem with a pattern like Singleton being in the GOF book is that people will, and do, use those patterns as a guide to best practice, and the "global point of access" part of Singleton is not (IMHO) good practice.
The intent of Mediator is to "Define an object that encapsulates how a set of objects interact". I don't have a problem with the intent, but rather with how I've seen Mediator used in practice. People use it as an excuse to not think about the responsibilities of objects, and just dump code in something they call a Mediator, resulting in very non object-oriented code. You may think it's unfair of me to object to a pattern because of it's misuse rather than correct use, but as with Singleton, I'd like a book that I can recommend to people where I don't see them use it as an excuse to create a mess. "We didn't know what to do with this code, so we applied the Mediator pattern, which must be good because it's in the GOF book".
(ooohhh - I can almost hear the gasps of disbelief; why could anyone want to remove this beautiful pattern?) The intent of Visitor is to "Represent an operation to be performed on the elements of an object structure". Many people confuse Visitor with "internal iterators". I'll try to explain using an example based on that in the GOF book, implemented in Java. Consider abstract syntax tree sort of thing, with an interface Node, and classes implementing that OperatorNode, NumberNode and ExpressionNode. The tree representing (3*2)+5 (here called "theTree") would be:
Node threeTimesTwo = new ExpressionNode(new NumberNode(3), new OperatorNode("*"), new NumberNode(2));
Node theTree = new ExpressionNode(threeTimesTwo, new OperatorNode("+"), new NumberNode(5));
(Full code provided at the end)
Let's say you want to pretty print this as "((3*2)+5)". Here are four different ways of implementing that:
You could ignore object-oriented programming and hack it together something like:
private static String theLeastOOPrettyPrint(Node root) {
StringBuffer result = new StringBuffer();
if (root instanceof ExpressionNode) {
result.append("(");
result.append(theLeastOOPrettyPrint(((ExpressionNode) root).getLeft()));
result.append(((ExpressionNode) root).getOperator().getOperator());
result.append(theLeastOOPrettyPrint(((ExpressionNode) root).getRight()));
result.append(")");
}
if (root instanceof NumberNode) {
result.append(Integer.toString(((NumberNode) root).getSomeNumber()));
}
if (root instanceof OperatorNode) {
result.append(((OperatorNode) root).getOperator());
}
return result.toString();
}
You could implement it using object-oriented programming as:
private static String theMostOOPrettyPrint(Node root) {
StringBuffer result = new StringBuffer();
root.prettyPrintOnto(result);
return result.toString();
}
and a method on OperatorNode (where operator is the string representation of the operator):
public void prettyPrintOnto(StringBuffer buffer){
buffer.append(operator);
}
and a method on NumberNode (where someNumber is the int represented by this Node):
public void prettyPrintOnto(StringBuffer buffer){
buffer.append(Integer.toString(someNumber));
}
and a method on ExpressionNode (where left, right and operator are the nodes representing the left, right and operator parts of the expression represented by this node. For example, for the ExpressionNode "theTree" defined earlier, left is the ExpressionNode "threeTimesTwo", operator is the OperatorNode representing "+" and right is the NumberNode representing "5"):
public void prettyPrintOnto(StringBuffer buffer){
buffer.append("(");
left.prettyPrintOnto(buffer);
operator.prettyPrintOnto(buffer);
right.prettyPrintOnto(buffer);
buffer.append(")");
}
Another alternative is using an "internal iterator" :
private static String iteratorPrettyPrint(Node root) {
final StringBuffer result = new StringBuffer();
root.each(new NodeInternalIterator(){
public void each(Node node){
node.prettyPrintOnlyYourself(result);
}
});
return result.toString();
}
where the "each" method on the "root" Node takes an instance of NodeInternalIterator and calls it's "each" method with each node in the tree as a parameter. The implementations of the iterating infrastructor are in the code included at the end. The "prettyPrintOnlyYourself" method implementations are the same as the "prettyPrintOnto" except for the case of the ExpressionNode, which becomes slightly trickier because it is now only responsible for it's own representation and not it's composite representation, and because it gets asked for it's representation after it's "children" :
public void prettyPrintOnlyYourself(StringBuffer buffer) {
buffer.insert(0,"(");
buffer.append(")");
}
That is, it now surrounds whatever is already on the buffer with braces.
In the "visitor" implementation, the logic is in the Visitor rather than the Node classes (again, the supporting infrastructor is in the code included at the end). In this case, the "accept" method on the "root" Node takes an instance of NodeVisitor and calls the appropriate "visitXXX" method with each node in the tree as a parameter, i.e. it calls visitOperatorNode for every OperatorNode, visitNumberNode for every NumberNode and visitExpressionNode for every ExpressionNode:
private static String visitorPrettyPrint(Node root) {
final StringBuffer result = new StringBuffer();
root.accept(new NodeVisitor(){
public void visitOperatorNode(OperatorNode node){
result.append(node.getOperator());
}
public void visitNumberNode(NumberNode node){
result.append(Integer.toString(node.getSomeNumber()));
}
public void visitExpressionNode(ExpressionNode node){
result.insert(0,"(");
result.append(")");
}
});
return result.toString();
}
People often call the "internal iterator" a "Visitor", but it isn't really a Visitor according to the GOF definition. A Visitor uses "double dispatch" to call a different method according to the type of node in the tree, in comparison to the "internal iterator" which calls the same method on every node in the tree.
The reason I'd like Visitor to be relegated to a "volume 2" of the GOF book is because it's not very object-oriented and is rather specialised. The logic is in the Visitor rather than the Node classes. I have seen Visitor used when the simple "object-oriented" solution would have been fine. That's not to say I don't see value in Visitor - it's a really neat pattern. There are cases where Visitor is the best choice of implementation, but it really is quite a specialised pattern that I don't think belongs in a "volume 1" of the GOF book. James Noble suggested that "double dispatch" should be included instead of Visitor, as Visitor is just an application of "double dispatch".
I'd like to thank Adewale Oshineye and Stacy Curl for their help with this article.
The full code used in this follows:
Interface Node:
package com.oocode.visitor;
public interface Node {
void prettyPrintOnto(StringBuffer buffer);
void each(NodeInternalIterator nodeIterator);
void accept(NodeVisitor nodeVisitor);
void prettyPrintOnlyYourself(StringBuffer result);
}
Class OperatorNode:
package com.oocode.visitor;
public class OperatorNode implements Node {
private String operator;
public OperatorNode(String operator) {
this.operator = operator;
}
public String getOperator() {
return operator;
}
public void prettyPrintOnto(StringBuffer buffer){
buffer.append(operator);
}
public void prettyPrintOnlyYourself(StringBuffer buffer) {
prettyPrintOnto(buffer);
}
public void each(NodeInternalIterator nodeIterator){
nodeIterator.each(this);
}
public void accept(NodeVisitor nodeVisitor){
nodeVisitor.visitOperatorNode(this);
}
}
Class NumberNode:
package com.oocode.visitor;
public class NumberNode implements Node {
private int someNumber;
public NumberNode(int someNumber) {
this.someNumber = someNumber;
}
public int getSomeNumber() {
return someNumber;
}
public void prettyPrintOnto(StringBuffer buffer){
buffer.append(Integer.toString(someNumber));
}
public void prettyPrintOnlyYourself(StringBuffer buffer) {
prettyPrintOnto(buffer);
}
public void each(NodeInternalIterator nodeIterator){
nodeIterator.each(this);
}
public void accept(NodeVisitor nodeVisitor){
nodeVisitor.visitNumberNode(this);
}
}
Class ExpressionNode:
package com.oocode.visitor;
public class ExpressionNode implements Node {
private Node left;
private OperatorNode operator;
private Node right;
public ExpressionNode(Node left, OperatorNode operator, Node right) {
this.left = left;
this.operator = operator;
this.right = right;
}
public Node getLeft() {
return left;
}
public OperatorNode getOperator() {
return operator;
}
public Node getRight() {
return right;
}
public void prettyPrintOnto(StringBuffer buffer){
buffer.append("(");
left.prettyPrintOnto(buffer);
operator.prettyPrintOnto(buffer);
right.prettyPrintOnto(buffer);
buffer.append(")");
}
public void prettyPrintOnlyYourself(StringBuffer buffer) {
buffer.insert(0,"(");
buffer.append(")");
}
public void each(NodeInternalIterator nodeIterator){
left.each(nodeIterator);
operator.each(nodeIterator);
right.each(nodeIterator);
nodeIterator.each(this);
}
public void accept(NodeVisitor nodeVisitor){
left.accept(nodeVisitor);
operator.accept(nodeVisitor);
right.accept(nodeVisitor);
nodeVisitor.visitExpressionNode(this);
}
}
Interface NodeInternalIterator:
package com.oocode.visitor;
public interface NodeInternalIterator {
void each(Node node);
}
Interface NodeVisitor:
package com.oocode.visitor;
public interface NodeVisitor {
void visitOperatorNode(OperatorNode node);
void visitNumberNode(NumberNode node);
void visitExpressionNode(ExpressionNode node);
}
Class PrettyPrint, which is where the different implementation techniques are used (a bit ugly, but adequate for my purposes):
package com.oocode.visitor;
public class PrettyPrint {
public static void main(String args[]) {
Node threeTimesTwo = new ExpressionNode(new NumberNode(3), new OperatorNode("*"), new NumberNode(2));
//the root node represents (3*2)+5
Node root = new ExpressionNode(threeTimesTwo, new OperatorNode("+"), new NumberNode(5));
// should produce ((3*2)+5)
System.out.println(theLeastOOPrettyPrint(root));
System.out.println(theMostOOPrettyPrint(root));
System.out.println(iteratorPrettyPrint(root));
System.out.println(visitorPrettyPrint(root));
}
private static String theLeastOOPrettyPrint(Node root) {
StringBuffer result = new StringBuffer();
if (root instanceof ExpressionNode) {
result.append("(");
result.append(theLeastOOPrettyPrint(((ExpressionNode) root).getLeft()));
result.append(((ExpressionNode) root).getOperator().getOperator());
result.append(theLeastOOPrettyPrint(((ExpressionNode) root).getRight()));
result.append(")");
}
if (root instanceof NumberNode) {
result.append(Integer.toString(((NumberNode) root).getSomeNumber()));
}
if (root instanceof OperatorNode) {
result.append(((OperatorNode) root).getOperator());
}
return result.toString();
}
private static String theMostOOPrettyPrint(Node root) {
StringBuffer result = new StringBuffer();
root.prettyPrintOnto(result);
return result.toString();
}
private static String iteratorPrettyPrint(Node root) {
final StringBuffer result = new StringBuffer();
root.each(new NodeInternalIterator(){
public void each(Node node){
node.prettyPrintOnlyYourself(result);
}
});
return result.toString();
}
private static String visitorPrettyPrint(Node root) {
final StringBuffer result = new StringBuffer();
root.accept(new NodeVisitor(){
public void visitOperatorNode(OperatorNode node){
result.append(node.getOperator());
}
public void visitNumberNode(NumberNode node){
result.append(Integer.toString(node.getSomeNumber()));
}
public void visitExpressionNode(ExpressionNode node){
result.insert(0,"(");
result.append(")");
}
});
return result.toString();
}
}
Martin Fowler missed out the Most Important Refactoring in his book - delete.
Before applying any of the refactorings in Martin Fowler's catalog, consider one of the delete refactorings. Sometimes, you will find that rather than renaming some badly named method (for example) you can just delete it, which is a much better refactoring.
One of the goals of refactoring is to reduce code entropy - and delete is the best refactoring for this. Less code means: less code to understand, less code to change when you make a change to something it depends on, less code to read through to find what you are looking for, shorter build times, shorter checkout times, shorter search times.
Mike Hill and I have put together a preliminary catalog:
You have code that isn't executed by the deployed system
Examples are: as-yet unrequested functionality, or functionality that is no longer required. Don't delete test or build code related to the deployed system.
You have code that is executed by the deployed system but does not effect the desired functionality of the system.
Examples are: EJBs for systems that don't need them, properties of objects that are set but never read.
You have code that is executed by the deployed system but has not been tested.
If the code hasn't been tested then you don't know what it does, so it is a risk to include it in the deployed system.
You have code that is difficult to maintain.
For example, code that no one understands, code that requires the same changes in multiple places for fixing a bug. Sometimes, you are better deleting it and starting again - but be careful not to jump to this solution too soon, it's often possible to refactor poor quality code into a usable state. (To be fair to Martin Fowler, I think he does mention this one).
Sometimes you need to retrofit unit tests to existing code that doesn't have unit tests, in order to fix it, modify it or refactor it.
This can be difficult, because code that has not been developed with unit tests will sometimes have dependencies that mean that tests written against the code as it stands would also test the code that it depends upon.
This often results in having to set up lots of dependencies which can take programming time and often make the tests run slower too.
This article describes one particular scenario and several different ways of retrofitting tests to code that has dependencies. These different approaches provide different trade-offs to get the immediate job done (e.g. a bug fix) verses long term maintainability.
This article assumes an understanding of Mock Objects12 but isn't really about Mock Objects. This article has bits of Dependency Injection3 in it too, but isn't about that either. It's about how to add tests to untested code that you need to fix.
I will present a few different options and won't make a blanket statement about which one is "best". It's a judgement call about balancing the effort of making the test code look good, making the non-test code look good, not changing too much of the non-test code (e.g. because the test coverage is very poor so making any changes is risky), and getting the job done, e.g. getting the test written that helps you fix some bug.
Consider the following Java class:
public class Legacy {
public void doSomething(){
Dependency dependency = new Dependency(someConstructionValue());
dependency.foo(someParameter());
}
...
where someConstructionValue() and someParameter() are private methods or some code that could be extracted into methods.
The code and names are rather abstract, as the approaches described here are applicable in many different circumstances.
The creation of the dependency object could equally be by a static method call, e.g:
"Singleton.createDependency(someConstructionValue())" instead of:
"new Dependency(someConstructionValue())" - for the purposes of this article they are the same, however, I'll stick with the constructor.
Examples could be where the Dependency is an EJB or a Database.
Let's say you've found a bug where the wrong value of something is being put in a database. You track down the bug and find that the value of "someParameter()" is wrong in some circumstances; that gets passed into the Dependency (lets say an EJB) which then causes it to put the wrong value in the database. To write a test without isolating the dependencies, you would need to be able to set up a Legacy object in the same state that caused the bug and then check what the value was in the database. There are lots of problems with this. If the test fails, you can't be sure where the problem lies - whether it's in the Legacy class or in the Dependency. Using the EJB/Database layers will make the test run slowly too.
All you really want to test is that the Dependency is passed the correct value - you can have other tests for the Dependency as to what it does when it is passed the correct value. For the rest of this article, I won't be considering what you need to do to set up the Legacy object; rather, just it's dependencies.
For the bug described earlier, you have a few options. You could make the method someParameter() public and write a test calling that directly. This is simple and quick to do, but exposes a method that would otherwise be private. Furthermore, you only know that the
value of this method will be used by reading the code - the test doesn't confirm that the value from someParameter() is actually passed into the dependency object. Nevertheless, it's quick and might get the job done. You might not be proud doing that but if it fixes some critical bug really quickly you might be best doing the fix by writing a test this way and implementing a better solution later.
Maybe better would be to test that the correct object is passed in to the call on the dependency object using a Mock Object. However, in this method, we have a problem: the Dependency object is created within the method so we can't subsitute a Mock Object for the constructed object.
There are several ways around this.
One approach would be to change the code to:
public void doSomething(Dependency dependency){
dependency.foo(someParameter());
}
public void doSomething(){
Dependency dependency = new Dependency(someConstructionValue());
doSomething(dependency);
}
This means you can pass in a Mock Object version of the dependency to doSomething(Dependency) and write the test against that method. Again, this has the problem of exposing a method just for testing, and there is no guarantee that doSomething() calls doSomething(Dependency). To test using a Mock Object, doSomething(Dependency) would be called by the test, passing in a Mock Object that implements the Dependency interface, and the test would be that the dependency expects to have the foo method called with the correct parameter (see 1 and 2).
Many options depend on whether someConstructionValue() is a fixed value or not and whether the Dependency object is stateful.
If the value of someConstructionValue() is a fixed value and the Dependency object is not stateful, you can pass in the Dependency object on construction, which keeps the interface of Legacy the same but allows for substituting the Dependency with a Mock Object.
public class Legacy {
Dependency dependency;
public Legacy(Dependency dependency){
this.dependency = dependency;
}
public void doSomething(){
dependency.foo(someParameter());
}
Other options for Dependency Injection are covered in3. If Legacy used to have a no-arg constructor, you either have to change the code that does that construction, or in this case you might consider the slightly ugly option of having two constructors, the one taking a Dependency for testing and a no-arg one:
public Legacy(){
this(new Dependency(someConstructionValue()));
}
How about if someConstructionValue() isn't fixed so you can't just create the dependency on construction? The "Local Injection" approach described earlier still works.
If you want to use a Dependency Injection style approach, you could pass in a mock DependencyFactory. In the real construction of a Legacy object, it would use a real DependencyFactory with a very simple implementation:
public class RealDependencyFactory implements DependencyFactory{
public Dependency newDependency(Object someConstructionValue){
return new Dependency(someConstructionValue);
}
}
and Legacy becomes:
public class Legacy {
DependencyFactory factory;
public Legacy(DependencyFactory factory){
this.factory = factory;
}
public void doSomething(){
Dependency dependency = factory.newDependency(someConstructionValue());
dependency.foo(someParameter());
}
This does the job but adds quite a lot of code. Even worse, in order to write the test, you need to set up the Mock DependencyFactory to return a Mock Dependency, which is what you really want to set expectations on.
If the Dependency is part of your code base, and if it's essentially stateless, it might be that it's interface can be changed so the value that was passed in on construction could instead be passed in as parameter on the foo method. The code would then look like:
public class Legacy {
Dependency dependency;
public Legacy(Dependency dependency){
this.dependency = dependency;
}
public void doSomething(){
dependency.foo(someConstructionValue(), someParameter());
}
and the test could pass in a Mock Dependency with expectations on the someParameter value or even on the someConstructionValue too, which would be useful if this needs testing too.
If the "localize constructor parameter" approach isn't possible (e.g. the Dependency isn't part of your code base) you could create an adaptor to do that.
If the Dependency class is:
public class Dependency {
public Dependency(Object object) {
...
}
public void foo(Object object) {
...
}
}
Then you can create an adaptor class:
public class DependencyAdaptor {
public void foo(Object constructionValue, Object parameter) {
new Dependency(constructionValue).foo(parameter);
}
}
And Legacy becomes:
public class Legacy {
DependencyAdaptor dependency;
public Legacy(DependencyAdaptor dependency){
this.dependency = dependency;
}
public void doSomething(){
dependency.foo(someConstructionValue(), someParameter());
}
...
Instead of the "adaptor" approach, another possibility would be the following:
public class Legacy {
public void doSomething(){
Dependency d = newDependency();
d.foo(someParameter());
}
protected Dependency newDependency(){
return new Dependency(someConstructionValue());
}
Then, in your test code, you could create a subclass:
public class LegacyExtensionForTesting extends Legacy{
public Dependency mockDependency;
protected Dependency newDependency(){
return mockDependency;
}
and write tests against the subclass, setting the mockDependency and it's expectations. This might look terribly hacky, but will get the job done and doesn't impact the non-test code too much.
Many thanks to my ThoughtWorks colleagues: Owen Rogers, Jim Arnold, Chris Stevenson and Nat Pryce, for their comments on this article.