Nicolas Frankel is an IT consultant with 10 years experience in Java / JEE environments. He likes his job so much he writes technical articles on his blog and reviews technical books in his spare time. He also tries to find other geeks like him in universities, as a part-time lecturer. Nicolas is a DZone MVB and is not an employee of DZone and has posted 223 posts at DZone. You can read more from them at their website. View Full User Profile

Should you change your design for testing purposes?

01.23.2012
| 4776 views |
  • submit to reddit

As Dependency Injection frameworks go, the standard is currently CDI. When switching from Spring, one has to consider the following problem: how do you unit test your injected classes?

In Spring, DI is achieved through either constructor injection or setter injection. Both allow for simple unit testing by providing the dependencies and calling either the constructor or the desired setter. Now, how do you unit test the following code, which uses field injection:

public class MyMainClass {
 
  @Inject
  private MyDependencyClass dependency;
 
  // dependency is used somewhere else
  ...
}

Of course, there are some available options:

  1. you can provide a setter for the dependency field, just as you did in Spring
  2. you can use reflection to access the field
  3. you can use a testing framework that does the reflection for you (PrivateAccessor from JUnit addons or Powermock come to mind)
  4. you can increase the field visibility to package (i.e. default) and put the test case in the same package

Amazingly enough, when Googling through the web, the vast majority of unit testing field-injected classes code demonstrate the increased visibility. Do a check if you do not believe me: they do not display the private visibility (here, here and here for example). Granted, it’s a rather rethorical question, but what annoys me is that it’s implicit, whereas IMHO it should be a deeply thought-out decision.

My point is, changing design for testing purpose is like shooting yourself in the foot. Design shouldn’t be cheaply sold. Else, what will be the next step, changing your design for build purposes? Feels like a broken window to me. As long as it’s possible, I would rather stick to using the testing framework that enable private field-access. It achieves exactly the same purpose but at least it keeps my initial design.

 

From http://blog.frankel.ch/shoud-you-change-your-design-for-testing-purposes

Published at DZone with permission of Nicolas Frankel, 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

Wujek Srujek replied on Mon, 2012/01/23 - 6:08am

I also would use less restricted visibility if I had to (to default, i.e. package private), I consider using reflection for this really silly, and introducing a framework just for that purpose even more so. My biggest headache here is that you are looking for the problem in the wrong place. You are preaching clean design, but you are using field injection, which is the actual problem and a smell. Multiple blogs and even the specs mention this as a quick prototyping support and a way to make code less verbose for snippets and tutorials. In production code you should _never, ever_ do field injection. What does it buy you? Such injection is actually done when the instance is created, which is the pretty much the same time when a constructor is called, but it has a couple of main drawbacks: a) as you noticed, it is not as trivial to test the class as with constructor injection b) the fields cannot be final any more Change your design to constructor injection and get final fields as a bonus. Additionally, it is documented clearly what your class / bean / service requires to work correctly. I know setter injection is sometimes a must, and then you don't have final fields any more, but still, field injection is the worst option of them all.

Andrew Spencer replied on Mon, 2012/01/23 - 6:21am

Hi Nicolas,

To follow up from Wujek's comment, there was another post recently on DZone about this same subject. Like Wujek, the author concluded that you should always do constructor injection.  If we only had named constructor parameters in Java, I'd agree with that.  As it is... (I'm desperately trying not to mention Scala)

Mladen Girazovski replied on Mon, 2012/01/23 - 6:52am

I also agree that this subject keeps coming up, however, (C)DI in a JEE environnent is a bit different than the rest...

If you use constructor injection, what happens when the Bean is migrated to another server in the cluster?

The dependency that was previously injected (ie. DAOs, Entitymanger, etc. pp.) might not even exist on this server, or another instance exists for that server... the CDI spec imposes some restrictions to constructor injection for a reason, the only valid alternatives are setter injection and field injection, since the container must be able to replace previously injected dependencies after serializing a bean to another server in the cluster.

Setter injection is flexible but introduces additional methods to the code, filed injection requires some magic in tests...

Marek Dec replied on Mon, 2012/01/23 - 7:38am in response to: Wujek Srujek

> I know setter injection is sometimes a must, and then you don't have final fields any more, but still, field injection is the worst option of them all.

While the constructor injection is definitely a winner (even though it's got its drawbacks too), I don't really see any advantages of setter injection over the field injection. Could you elaborate on that, please?

Tech Fun replied on Mon, 2012/01/23 - 8:27am

Use constructor injection for the must-have dependencies, but use setter injection for the optional dependencies and fallback elegantly when they are not provided. Field injection should be prohibited. At first look it might be helpful, but it's hurting the readability: it does not express dependencies clearly.

Wujek Srujek replied on Mon, 2012/01/23 - 12:22pm in response to: Marek Dec

It's pretty simple - you have services whose dependencies cannot be injected once, during instantiation, but rather can be replaced during runtime. I am not arguing that this is great design; I am just saying that sometimes, for some reason, it might be wanted. For example our controller code does this all the time (I didn't write it, and I really hate the fact that it looks the way it does, with interfaces for each class and setters in the interface). The explanation from the UI team is that the objects must be pooled as they are expensive to create, and have their dependencies (like the current module shown in the application) replaced every now and then (like when the user switches modules). #1 on my hate list when it comes to the current project I am working on.

Erwin Mueller replied on Mon, 2012/01/23 - 12:48pm

I don't understand. If it's part of the requirements that some dependencies can or must be replaced, than it's needs to be a part of the design, too.

So if the requirement from the UI team is that some dependencies have to be replaced, which are expensive to create (like when the user switches modules) than why are you reluctant to add a setter method that clearly states the requirement in the design?

 For example, in Swing we have the same like JScrollPane#setHorizontalScrollBar. Normally I would say the horizontal scroll bar is a final-dependency, because without the JScollPane is useless. But there are requirements to change the scroll bar at run-time, so there is a setter method.

You can even easily  communicate the indent of the setter method:

/**
* Used by the UI team to replace the dependency if the user switches a module.
*/
@Inject
void setDependency(Dependency newDependency) { }

Sebastian Gozin replied on Mon, 2012/01/23 - 5:25pm

I don't really think private field inject is a big deal as your tests would communicate the dependency anyway. That said in a statically typed language like Java you will immediately bump in the "how to test this?" problem and force you to fix it. Your design was wrong and your tests pointed that out. Now make a design decision like other commenters have done.

Marek Dec replied on Tue, 2012/01/24 - 3:45am in response to: Wujek Srujek

@Wujek

 Okay, then it looks like the design of your app implies the existence of the setters. But would you still consider setter injection a better choice than field injection if your injection framework was capable of replacing the dependencies of the existing objects at runtime?

e.g.
MyFramework.take(anObjectFromPool).andInject(anotherDependency.class, yetAnotherDependency.class).

In other words, is it a good idea to change your design (namely: to create setters) in any case when using a dependency injection framework? Would you publish those setters via a public interface? What visibility would those setters get anyhow?
And what about the constructor injection pattern, should the ctors be public if their only explicit clients are tests (and the DI framework of course, via some extralinguistic mechanisms though)?

 

 

Ivan Lazarte replied on Tue, 2012/01/24 - 1:45pm

What an amazingly horrible way to bring in DI. The obvious answer is a getter/setter for the injected value. To not understand that is to have missed the entire POJO revolution.

Package visibility is brittle. Right now your class in package X, but one drag and drop later, it could be somewhere and the intent of that visibility is completely lost.

Comment viewing options

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