In general setter injection is frowned upon. The main reason is mostly that an object should have all its dependencies at the creation time. That is very reasonable, but I find it a tad on the paranoid side. Or better put, I have been using setter injection only and have not found any problem ever. Furthermore, by having setter injection my objects and test code has remained cleaner than the alternative options.
Here are the rules
- Depend only on interfaces
- Don't use new
- Don't define constructors in the class
Depending always on interfaces sounds extremist. But it happens naturally if a system is developed outside-in mocking the dependencies. The principal advantage of mocking is increased focus at the time of writing the code, the decoupledness is a side effect. Another side effect is that the interface becomes the public interface of the class.
Therefore making things public in the class doesn't break encapsulation. I do confess that having more public and virtual elements in the class obscures the understanding of what the class offers. But the interface is right there and it's easy to see what the class is exposing to others thru it.
Depending only on interfaces needs to be accompanied by forbidding the use of new. If an object has a dependency say "Command ToSave;" and does "ToSave = new SaveCommand();" to create it. The class would have a dependency on both the interface Command and the class SaveCommand.
Once new is forbidden there are two ways to create a class, either by having a factory object or by using a DI framework. Both approaches would lead to the decision of setter injection vs constructor injection. I chose setter because the class ends up cleaner that way by not needing constructor logic.
There's no way of generating an object on an unstable state because the dependencies are either injected by a framework or they had to be passed as args to the factory method. There's a backdoor to the object via new but it is simply forbidden and conveniently available to the test code.
The test code becomes simpler because we need to mock only the dependencies involved on a particular behavior. It is a smell to have multiple dependencies and only some of them needed at a given time but it happens. Even if it's only at some stage before the object is refactored into a better design.
Posts
I agree with you in almost all points but I still stand that in terms of intent, for instance, constructors are the language mechanism to use when you want to build an object.
ReplyDeleteIn terms of cleanliness, it appears that it is in the eye of the beholder because to me using the constructors instead of having my classes polluted with setter methods seems more clean.
You can argue that this policy (of using constructors only) is a bit paranoid. I would say that it is less error prone. But unit tests will mitigate this risk by a lot.
I have a compelling argument for the constructor injection which is immutability. Immutable objects are so must better to handle, test, copy and pass around.
I agree that it is a smell to have multiple dependencies and only use some of them but the problem doesn't not lye in the constructor of the object but in its responsibilities - it might be doing too much...
thank you for your response ... and your tweet that triggered the post :)
ReplyDeleteI forgot to mention I use C# and declare the dependencies as "public Command ToSave { get; set; }" ... I think my main objection with Java is not having properties ... so yes if I would still need to write a set/get method I would not gain anything
my unit tests would not help me much because the dependencies are *always* mocked ... my most common error between done and done done is a Null exception because I forgot to inject the dependency ... at that point all unit tests are green but I have a red acceptance test ... i'm always happy to be reminded that i didn't think of object construction at all
if I use a DI framework I could even auto inject dependencies based on conventions and I would almost not need to be reminded unless I'm in a special case ... but I haven't got there yet (laziness)
I haven't got too far finding good Value Objects in my designs ... got to improve there ... still I only depend on interfaces so the class is as immutable as its interface despite having public dependencies
SRP is very fuzzy ... I do find the smell of many dependencies a trigger unless it's a facade object ... but I was meaning more about the cohesion of the dependencies ... if they are all needed together the facade sounds to be following SRP
No worries, thanks for following it up and starting a nice conversation.
ReplyDeleteYes, I really meant integration tests. :)
I know next to nothing in C#, I can read code as the syntax is similar to Java and I'm familiar with concepts like delegates and some of the functional things that were put in recently in the language but I never wrote a single line.
Guice is a DI that allows you to do that as well.
As far as everyone knows that in concept the class is _immutable_ but in practice it is not and people don't misuse it, I guess it's fine. :)
Have a look at the DDD book and some talks in InfoQ about value objects. Now a days I can't live without them but some of it because of the lack of list comprehensions and closures in java.
About SRP, if you find that they are all needed together does it still make sense to only inject half the object?