However, like any good idea, if taken to the extreme it becomes counterproductive. If everything a moderately complex class did was abstracted and injected, you would end up with a confusing and incoherent jumble of tiny classes. You would also risk exposing too much of the internals of a class by requiring any consumer of that class to create and inject pieces unrelated to the behavior the consumer wishes to dictate.
Let's make a simple example. Suppose we had a component that resizes an image. But in order to complete its work, needs to create a temporary file. Let's first take a look at an implementation that doesn't use DI.
public class ImageResizer { public File resizeImage(File image) throws IOException { File tmp = File.createTempFile("tmp", null); // do work on tmp ... } }
Simple enough, but how are we going to test this? It uses a static method, which we don't own and can't change, to create the temporary file. We don't have any way to mock it or inspect it, so we're pretty much out of luck for testing it.
Now let's use a strict form of DI. We'll abstract the temporary file creation into a separate class, and require consumers to provide an implementation at construction time.
public class ImageResizer { /** Abstraction of temp file management */ public static class TempFileFactory { File createTempFile() throws IOException { return File.createTempFile("tmp", null); } } private final TempFileFactory fileFactory; /** Dependency-injection constructor */ public ImageResizer(TempFileFactory fileFactory) { this.fileFactory = fileFactory; } public File resizeImage(File image) throws IOException { File tmp = fileFactory.createTempFile(); // do work on tmp ... } }
Better. At least we can write a test to mock TempFileFactory, inject the mock into the ImageResizer, and validate the interactions between ImageResizer and the temporary file. But now we've burdened consumers of ImageResizer -- which simply want to resize a file -- with the requirement of managing temporary files (by creating a TempFileFactory; alternately we could have required consumers to inject a temporary file, which is probably even worse) and the awkward knowledge that ImageResizer uses temporary files. If we made a breakthrough in the ImageResizer so that it no longer needed to use a temporary file, all the consumers would need to change their code.
So how do we get the benefits of testability and isolation without this downside? We still embrace the concept of DI but use defaults to hide this from consumers, in what I call "Loose Dependency Injection"
public class ImageResizer { /** Package-private abstraction of temp file management */ static class TempFileFactory { File createTempFile() throws IOException { return File.createTempFile("tmp", null); } } private final TempFileFactory fileFactory; /** Public constructor, injects its own dependency */ public ImageResizer() { this.fileFactory = new TempFileFactory(); } /** Package-private constructor for use by test */ ImageResizer(TempFileFactory fileFactory) { this.fileFactory = fileFactory; } public File resizeImage(File image) throws IOException { File tmp = fileFactory.createTempFile(); // do work on tmp ... } }
Fundamentally we're still using DI; the difference is that there's only one implementation of the dependency, and it is "injected" by the default constructor. The consumer has no knowledge that the ImageResizer has anything to do with temporary files. ImageResizer could change to not use temporary files, and no client code would need to change. Tests for ImageResizer are easy to write because we can mock ImageResizer.TempFileFactory. The best of all worlds!
Nice. I guess in this case there probably isn't much performance penalty by doing it this way but I wonder if you could squeeze out a little extra optimization for consumers by moving the assignment from the public constructor to a static initializer block?
ReplyDeleteI think it would depend on the nature of the helper class. In this case we could do that, since the TempFileFactory is stateless. But if it were something which kept state, we'd need a new instance every time.
ReplyDeleteThis is a good callout though; we're paying an additional cost at construction time with this approach, so it should be used with caution in scenarios where we'd be creating the outer class in a tight loop or something.