Recently I have written that @Autowired annotation make our lives easier in that it allows us to write less code. However, using it may often make your design more complicated. Especially when we are talking about using it on class’s properties. It makes it easier to violate
Single Responsibility Principle. It makes it easier not to notice that.
It is ok to use @Autowired with properties, yet, I think, that presence of the constructor visualizes the problem with too many dependencies when it occurs. Even without exercising additional care from our side.
The code that grows
Let’s use a known example from the article about constructors and setters, but we will modify it a bit.
So, once upon a time there was an event that had to be handled. We firstly had to retrieve all required information from the database and after this we were able to trigger an appropriate action based on this information.
Based on the requirements we created the following snippet of code:
public class SomeHandler {
@Autowired private final Repository repository;
@Autowired private final Trigger trigger;
public void handle(SomeEvent event) {
// some code
}
}But change is the only thing that is certain. There were new demands from the customer and we had to extend the functionality. Customer wanted to store all the information, before proper action can be taken. They also wanted to be notified in case of any emergency event occurrence.
After all changes we ended up with something like this:
public class SomeHandler {
@Autowired private final Repository repository;
@Autowired private final Trigger trigger;
@Autowired private final SnapshotTaker snapshotTaker;
@Autowired private final Notifier notifier;
public void handle(SomeEvent event) {
// some code
}
}Is it well-written code from the design of the class perspective? Well, I believe that three-four dependencies should definitely give us the reason to start thinking about small redesign.
And four unfortunately isn’t the highest number of dependencies that I have ever seen in the code of the single class…
Where’s @Autowired in it?
Ok, but what @Autowired has to do about it? The solution to the problem is to refactor and redesign the code when needed, am I right? I believe that some of you will agree with me. However, before decision about any change could be taken, we had to spot the problem.
Ok, so once again – what @Autowired has to do about it? Isn’t it obvious that the number of the dependencies is kind of problematic? Well, using @Autowired on field makes it kind of a blur. The code above does not seem like a huge pain when you are looking at it. Those are only four lines of code, short lines of code. We may argue that any good developer should know when many is too many, but why we should assume when we can write the code that can express the problem itself?
If all of those dependencies are required we can use constructor to inject them:
public class SomeHandler {
private final Repository repository;
private final Trigger trigger;
private final SnapshotTaker snapshotTaker;
private final Notifier notifier;
@Autowired
public SomeHandler(Repository repository, Trigger trigger, SnapshotTaker snapshotTaker, Notifier notifier) {
this.repository = repository;
this.trigger = trigger;
this.snapshotTaker = snapshotTaker;
this.notifier = notifier;
}
public void handle(SomeEvent event) {
// some code
}
}Now the code “tells” us something. This long constructor’s declaration just does not look good. We don’t have to think whether four is too much or not. We see that.
And in the example we are using short names of the both classes and methods, but in real application those names are sometimes much longer to be as descriptive as possible:
@Autowired
public SomeHandler(EventRepository eventRepository, EventActionTrigger eventActionTrigger, EventSnapshotTaker eventSnapshotTaker, EmergencyIssueNotifier emergencyIssueNotifier) {
this.repository = eventRepository;
this.trigger = eventActionTrigger;
this.snapshotTaker = eventSnapshotTaker;
this.notifier = emergencyIssueNotifier;
}Now the problem is more visible, isn’t it?
And adding another dependency would just hurt ours eyes (if it has not happened already):
@Autowired
public SomeHandler(EventRepository eventRepository, EventActionTrigger eventActionTrigger, EventSnapshotTaker eventSnapshotTaker, EmergencyIssueNotifier emergencyIssueNotifier, SomeAnotherDependency someAnotherDependency) {
this.eventRepository = eventRepository;
this.eventActionTrigger = eventActionTrigger;
this.eventSnapshotTaker = eventSnapshotTaker;
this.emergencyIssueNotifier = emergencyIssueNotifier;
this.someAnotherDependency = someAnotherDependency;
}We need to go deeper!
But don’t stop at this moment. Let’s look at the code when we are extending a class:
public interface Handler {
void handle(Event event);
}
public abstract class BasicEventHandler {
@Autowired private final EventRepository eventRepository;
@Autowired private final EventActionTrigger eventActionTrigger;
// some code
}
public class SomeHandler extends BasicEventHandler implements Handler {
@Autowired private final EventSnapshotTaker eventSnapshotTaker;
@Autowired private final EmergencyIssueNotifier emergencyIssueNotifier;
@Autowired private final SomeAnotherDependency someAnotherDependency;
public void handle(SomeEvent event) {
// some code
}
}It the first example we can argue whether the number of the lines of code that declares the object’s dependencies is big enough to make us worry. When we are extending a class, this number in a particular class can be fine.
However, the problem with this code is even bigger than in the previous example.
The author is usually certain that they solution is good. This is the way how we, authors, treat what we are creating.
But this time even the reviewer of the code may not notice the problem. Unfortunately not everyone will take a look at the parent class. When code is in the review it requires additional effort. Even without checking out the branch we still have to go to our IDE and open the class. Maybe it is not so much, but believe me that for some of us it is too much.
There would be no issue to notice a problem if we agreed that the constructor is a required part of our classes:
public class SomeHandler extends BasicEventHandler implements Handler {
private final EventSnapshotTaker eventSnapshotTaker;
private final EmergencyIssueNotifier emergencyIssueNotifier;
private final SomeAnotherDependency someAnotherDependency;
@Autowired
public SomeHandler(EventRepository eventRepository, EventActionTrigger eventActionTrigger, EventSnapshotTaker eventSnapshotTaker, EmergencyIssueNotifier emergencyIssueNotifier, SomeAnotherDependency someAnotherDependency) {
super(eventRepository, eventActionTrigger);
this.eventSnapshotTaker = eventSnapshotTaker;
this.emergencyIssueNotifier = emergencyIssueNotifier;
this.someAnotherDependency = someAnotherDependency;
}
public void handle(SomeEvent event) {
// some code
}
}Make it obvious!
We can think about agreements in terms of how many is too many. We can try to follow our own rules. However, there is always someone who will challenge that. Who will write the code and will try to prove that everything fits together and should be together and this is not “too many” yet.
Don’t argue, let the code speak for itself!
| Reference: | @Autowired all the things! from our JCG partner Sebastian Malaca at the Let’s talk about Java blog. |
