I love Spring framework, Dependency Injection and everything it has done for mankind (or Java developers, at least). And I hope that it thrives for many more years and helps us write amazing software (or gives us the tools for it, at least).

However, more than a fair share of projects I have encountered in my career do have a tendency to abuse some or many tools the framework. I want to think that this is just a coincidence. And that there are Spring (Boot) projects out there that actually leverage the framework without making our OOP ancestors turn in their graves. I want to believe that there are projects out there with only one *Utils.java class. And that people still use component scanning instead of explicitly declaring their beans. I want to believe in projects without @Autowired mayhem in each and every class. Btw, if you are one of those people abusing autowiring in your project, open your IDE now and replace each of them with constructor injection and punch yourself in the face for each @Autowired. You can thank me later.

Okay, I got carried away a bit there so let’s get to the point. Ready?

You don’t have to use Dependency Injection to solve every problem in a Spring application.

Even though this is very easy and tempting to do, let’s think before using our hammer to bend a wire (maybe pliers will do?). If this has just blown your mind and you are in a state of shock, don’t worry, I will explain with some examples soon. Just sit down, wipe off that cold sweat from disbelief and keep reading.

Let’s take an every day generic Spring Boot application that deals with some user registration and see how it evolves over time and when we should file for Dependency Injection abuse to the Code Police.

The beginnings of every project: DI Grenfield Bliss

DI Greenfield Bliss

Let’s say our project has just began, we are writing an MVP (that will, eventually end up in production, of course) with a set of simple features that might look like this:

@RestController
public class UserRegistrationController {

    final UserRegistrationService service;

    public UserRegistrationController(UserRegistrationService service) {
        this.service = service;
    }
    
    @GetMapping("/user/{id}")
    public User getUser(@PathVariable String id) {
        return service.getUserById(id);
    }
    
    @PostMapping("/user")
    public User createUser(@RequestBody User user) {
        return service.create(user);
    }
    
    // more of the usual stuff...
}

public class UserRegistrationService {

    private UserRegistrationRepository repository;

    public UserRegistrationService(UserRegistrationRepository repository) {
        this.repository = repository;
    }

    public User create(User user) {
        return repository.save(user);
    }

    public User getUserById(String id) {
        return repository.read(id);
    }
   // more of the usual stuff....
}

Pretty neat, right? Just what you would get in a tutorial, all perfectly fits together. Now let’s fast forward possibly 3 – 6 months down the project roadmap, depending on how fast you are delivering and getting new requirements but pretty soon, the code starts morphing into something hideous…

The steady decline: DI Junkie + procedural OOP

Just give me Autowired annotations and I can conquer the world with Dependency Injection

@RestController
public class UserRegistrationController {

    final UserRegistrationService userRegistrationService;
    final PremiumUserRegistrationService premiumUserRegistrationService;
    final DiamondUserRegistrationService diamondUserRegistrationService;
    public static final String PREMIUM_TIER = "premium";
    public static final String DIAMOND_TIER = "diamond";

    public UserRegistrationController(UserRegistrationService userRegistrationService,
                                      PremiumUserRegistrationService premiumUserRegistrationService,
                                      DiamondUserRegistrationService diamondUserRegistrationService) {
        this.userRegistrationService = userRegistrationService;
        this.premiumUserRegistrationService = premiumUserRegistrationService;
        this.diamondUserRegistrationService = diamondUserRegistrationService;
    }

    @GetMapping("/user/{tier}/{id}")
    public User getUser(@PathVariable String tier, @PathVariable String id) {
        if (PREMIUM_TIER.equals(tier)) {
            return premiumUserRegistrationService.getUserById(id);
        } else if (DIAMOND_TIER.equals(tier)) {
            return diamondUserRegistrationService.getUserById(id);
        }
        return userRegistrationService.getUserById(id);
    }

    @PostMapping("/user/{tier}")
    public User createUser(@RequestBody User user, @PathVariable String tier) {
        if (PREMIUM_TIER.equals(tier)) {
            return premiumUserRegistrationService.create(user);
        } else if (DIAMOND_TIER.equals(tier)) {
            return diamondUserRegistrationService.create(user);
        }
        return userRegistrationService.create(user);
    }

“Hey, I know I should not put any logic in my controller but it was the fastest way to do it because we had this tight deadline and I was working on another issue so let’s just merge this now and we can refactor later…” (Spoiler alert: Later never happened!).

public class UserRegistrationService {

    private UserRegistrationRepository repository;
    private SpecialMemberClient specialMemberClient;
    private UserHistoryClient userHistoryClient;
    private UserMembershipValidator validator;
    @Value("${feature-flags.discount:false")
    private boolean discountEnabled;
    @Value("${feature-flags.blacklist:false")
    private boolean blacklistEnabled;

    public UserRegistrationService(UserRegistrationRepository repository, SpecialMemberClient specialMemberClient,
                                   UserHistoryClient userHistoryClient, UserMembershipValidator validator) {
        this.repository = repository;
        this.specialMemberClient = specialMemberClient;
        this.userHistoryClient = userHistoryClient;
        this.validator = validator;
    }

    public User create(User user) {
        validator.validateUser(user);
        user = userHistoryClient.checkIfResubscribing(user);
        if (discountEnabled) {
            applyDiscount(user);
        }
        String specialMemberId = specialMemberClient.checkIfSpecialMember(user.getEmail(), user.getCardInfo());
        user.setSpecialMemberId(specialMemberId);

        return repository.save(user);
    }

    public User getUserById(String id) throws UserBlacklistedException {
        if (blacklistEnabled) {
            checkBlacklist(id);
        }
        return repository.read(id);
    }

    // the already unusual stuff...
}

I know more than a few who would not give an “LGTM!” for a PR with that code. It’s easy to just continue spawning new classes, use DI to wire them up where needed and pass the data. The problem is, in 2020 that 80% of today’s web applications are “microservices” so you can get away with it sometimes and you will not feel immediate pain for running that in production and having to maintain/extend it. But the awful truth is that the code above does not scale. Eventually you will have God classes and start suppressing Sonar screams for excessive imports. And everything that we cannot solve with adding a new dependency – we’ll just branch around a boolean flag or a null, that should work, right?

Bottomless pits of Tech Debt: DI Hell and “let’s use Java 8”

As the time goes by and the project grows, one day you wake up and see something like this:

Spaghetti code chaos and one letter variables that make 0 sense

If you have been here before, you know how hard it can be. Let’s look at this in code:

public class UserRegistrationService {

    // first 3 layers of DI hell...
    
    private UserSearchEntity findUserSearchEntity(final String userType, final String region, final Date date,
                                                  final List<UserAttributes> userAttributes,
                                                  final boolean cacheable) {
        final int userTypeId = userTypeService.getUserTypes().get(userType).getId();
        final RegionEntity region = regionService.getRegions().get(region);
        final int regionId = region.getId();

        final List<AttrIdValueKey> attrs = getAttributesList(userType, userAttributes, region);

        final Map<AttrIdValueKey, List<UserEntity>> userMap = cacheable ?
                userDataService.loadCachableUserMapForRegion(regionId)
                : userDataService.loadUserMapForRegion(regionId);

        final Comparator<Pair<OrgIdValueKey, UserEntity>> comparingBySortOrder =
                Comparator.comparing(v -> v.getSecond().getSortOrder());

        final List<Pair<AttrIdValueKey, PageRuleEntity>> usersReduced = attrs.stream()
                .map(key -> {
                    final List<UserEntity> rKeep on hackingule = userMap.get(key);
                    return rule == null ? null : Pair.of(key, rule);
                })
                .filter(Objects::nonNull)
                .flatMap(p -> p.getSecond().stream().map(l -> Pair.of(p.getFirst(), l)))
                .filter(p -> p.getSecond().isActive()
                        && p.getSecond().getUserType().getId() == pageTypeId
                        && p.getSecond().getRegions().stream().anyMatch(r -> r.getId() == regionId)
                        && p.getSecond().getLaunchDate().getTime() <= date.getTime()
                        && (p.getSecond().getEndDate() == null
                        || p.getSecond().getEndDate().getTime() >= date.getTime()))
                .sorted(comparingBySortOrder.thenComparing(v -> v.getSecond().getId()))
                .collect(Collectors.toList());

        final UserEntity matchingUser = findMatchingUser(attrs, usersReduced);
        return matchingUser == null ? null
                : new UserSearchEntity(matchingUser.getId(),
                matchingUser.getOriginUserId(),
                matchingUser.getEmail(),
                matchingUser.getPhone());
    }
    
    // the rest of the ugly code...
}

I gave you just one example method from one of the classes above due to the fact both of them ended up over 300 lines long and the ugly code will scar your eyes and plague your brain for life. But you get the idea, right. Just classes injected with more classes that do things in a procedural manner and some lambda expressions with side effects.

The inevitable moment of reckoning: Cruft payback

Then it happens, one morning you are debugging your code for an elusive issue that triggered a major outage the night before and woken you up for a 1 hour ghost chase that ended up in a rollback to previous version. After several coffees, more than a few WTFs! said too loud and dozens of breakpoints in your code, you finally find it! Shaking your head in disbelief, you do a git blame to see what genius has written this piece of code and then it hits you…

The irony of being disgusted by your own code

Conclusion: Never abuse Dependency Injection

This goes in general for anything but dependency injection is most misused in today’s OOP due to being so accessible from various frameworks (wink, wink). Invest time in designing your code and it will definitely pay off in positive ways. If you don’t invest in cleaning up the cruft from your code, it will definitely come back to bite you (and probably in the middle of the night).

Sorry about all the bitchin’ and preachin’ in this post but I had to get this off my chest as I was seeing a lot of ugly code today but I will definitely continue these series with a more optimistic note when I start hacking into the cruft of it and refactor each class out of it’s misery into a more bright OO future with less dependencies.