Autowired annotation from Spring Framework is a wonderful thing. It makes a development of complex applications pretty easy, objects appear in your class automagically! You just splat an annotation on a field and voila! Magic happens. But it’s not a magic bullet that you should plug in everywhere.

Recently I have started working on a new project with existing code base. So while browsing and reading classes, interfaces etc. I stumbled upon an interesting spring Service class. The class wasn’t big, about 600 lines of code, so not bad, but could be a bit smaller for my taste. Anyway, what caught my attention was the amount of Autowired private fields. It had 18 of them. When I saw this, I thought one thing, "WTF", except it was in my native language with words that I won’t post publicly online 😉

You may ask, what’s wrong with that? It works, so what’s the problem?

Use constructor

First of all, you shouldn’t use @Autowire on fields. Use @Autowire on the constructor. If these dependencies are really required by the class, then you should not allow to instantiate the class without them. The correct way to do this is by using constructor parameters. If some of these parameters are optional, then these can be annotated as fields and marked as optional. But most of the time, when people use Autowired they think of dependencies that are required.

So instead of doing this:

@Service public class MyService { @Autowired private ServiceDependency1 serviceDependency1; @Autowired private ServiceDependency2 serviceDependency2; @Autowire private ServiceDependency3 serviceDependency3; } 1 2 3 4 5 6 7 8 9 10 11 12 @Service public class MyService { @Autowired private ServiceDependency1 serviceDependency1 ; @Autowired private ServiceDependency2 serviceDependency2 ; @Autowire private ServiceDependency3 serviceDependency3 ; }

Do this:

@Service public class MyService { private ServiceDependency1 serviceDependency1; private ServiceDependency2 serviceDependency2; private ServiceDependency3 serviceDependency3; @Autowired public MyService(ServiceDependency1 serviceDependency1, ServiceDependency2 serviceDependency2, ServiceDependency3 serviceDependency3) { this.serviceDependency1 = serviceDependency1; this.serviceDependency2 = serviceDependency2; this.serviceDependency3 = serviceDependency3; } } 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 @Service public class MyService { private ServiceDependency1 serviceDependency1 ; private ServiceDependency2 serviceDependency2 ; private ServiceDependency3 serviceDependency3 ; @Autowired public MyService ( ServiceDependency1 serviceDependency1 , ServiceDependency2 serviceDependency2 , ServiceDependency3 serviceDependency3 ) { this . serviceDependency1 = serviceDependency1 ; this . serviceDependency2 = serviceDependency2 ; this . serviceDependency3 = serviceDependency3 ; } }

Unit tests

Another negative aspect of @Autowired on fields we can see in unit tests. When you write a test you have to instantiate your class. When you have private, autowired fields, you have to set them, probably using setter if you have one defined, but if you don’t, then well, you can’t test your class anyway. Of course, setting your mocks with setters is not bad, the problem with that is that you have no idea which field you have to set and which field you don’t. With constructor parameters, you know exactly what you have to provide. And if a new dependency is added, you will know about this at compile time, not from failing tests (if you are lucky).

Don’t do this:

public class ServiceTest { @Test public void someTest() { ServiceDependency1 serviceDependency1 = mock(ServiceDependency1.class); ServiceDependency2 serviceDependency2 = mock(ServiceDependency2.class); ServiceDependency3 serviceDependency3 = mock(ServiceDependency3.class); MyService service = new MyService(); service.setServiceDependency1(serviceDependency1); service.setServiceDependency2(serviceDependency2); // what if you forget to set this one? ;) service.setServiceDependency3(serviceDependency3); // your test code } } 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 public class ServiceTest { @Test public void someTest ( ) { ServiceDependency1 serviceDependency1 = mock ( ServiceDependency1 . class ) ; ServiceDependency2 serviceDependency2 = mock ( ServiceDependency2 . class ) ; ServiceDependency3 serviceDependency3 = mock ( ServiceDependency3 . class ) ; MyService service = new MyService ( ) ; service . setServiceDependency1 ( serviceDependency1 ) ; service . setServiceDependency2 ( serviceDependency2 ) ; // what if you forget to set this one? ;) service . setServiceDependency3 ( serviceDependency3 ) ; // your test code } }

Do this:

public class ServiceTest { @Test public void someTest() { ServiceDependency1 serviceDependency1 = mock(ServiceDependency1.class); ServiceDependency2 serviceDependency2 = mock(ServiceDependency2.class); ServiceDependency3 serviceDependency3 = mock(ServiceDependency3.class); MyService service = new MyService(serviceDependency1, serviceDependency2, serviceDependency3); // your test code } } 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 public class ServiceTest { @Test public void someTest ( ) { ServiceDependency1 serviceDependency1 = mock ( ServiceDependency1 . class ) ; ServiceDependency2 serviceDependency2 = mock ( ServiceDependency2 . class ) ; ServiceDependency3 serviceDependency3 = mock ( ServiceDependency3 . class ) ; MyService service = new MyService ( serviceDependency1 , serviceDependency2 , serviceDependency3 ) ; // your test code } }

Number of parameters

Let’s say that you have all your @Autowired parameters in the constructor, all 18 of them. The first problem of this is that it is completely unreadable. It probably takes your whole screen space just to display constructor declaration, and a situation like that is a code smell, a really bad smell.

In my opinion, we should treat constructor like any other method. Same rules apply. Here’s a little excerpt from Clean Code by Robert C. Martin:

The ideal numbers of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification ‐ and then shouldn’t be used anyway.

As you see, we should aim to limit the number of arguments as much as possible, aim for 3 at maximum, use 4 if you really must. But what if you have more dependencies? Like 18 of them? Well, in a case like that, you should probably think more about your class. It’s a hint that, maybe, just maybe, your class is doing too much? Think about SOLID principles, and especially about Single Responsibility Principle:

The single responsibility principle is a computer programming principle that states that every module or class should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by the class. All its services should be narrowly aligned with that responsibility.

So please, think about your design, about the structure of your class, maybe it is doing too much. Maybe some of these dependencies can be grouped together somehow? Think why you need them in your class, how they relate to each other, how you can close some of those relations in some other class. Most of the time there is a solution for that, you just have to stop for a moment and think.

If you don’t, then well, you will have to mock all 18 of them in your tests! Have fun with that!

Also published on Medium.