Field injection in quarkus

[BrianH85]

From Sonarqube we have been getting code issues concerning field injection. For example the article here describes it: https://www.baeldung.com/java-spring-field-injection-cons

Are some or all of these concerns also valid when using Quarkus field injection? I'm trying to figure out if and when we should use constructor injection instead.

[quarkus-bot[bot]]

/cc @Ladicek (arc), @manovotn (arc), @mkouba (arc)

[Ladicek]

The concerns in the linked article generally apply to all full-featured dependency injection containers, including CDI. From a purist perspective, constructor injection should obviously be preferred to field injection. On the other hand, field injection is immensely pragmatic (what's the point of writing a constructor that just assigns its parameters to fields?). Specifically when it comes to testing and mocking, Quarkus has multiple tools that make testing a breeze even in presence of field injection (see https://quarkus.io/guides/getting-started-testing).

[manovotn]

Ladislav is right on the money.
I'd just add that for some beans in CDI (normal-scoped beans - req. scoped, app. scoped) you will need to provide a no-args constructor in order to be able to create client proxy anyway which makes the argument for constructor injection somewhat weaker.
Furthermore, CDI also allows you to use initializer methods which is yet another way of setting up your fields.

[Ladicek]

Note that what CDI calls initializer methods is elsewhere called setter injection.

[mkouba]

And for a good reason - an initializer method in CDI may accept multiple params, so it's not a classic "setter" ;-).

[edeandrea]

To be honest I always prefer constructor injection over field injection. It makes it explicitly clear what fields are required to instantiate a class and therefore make it useful. You have no idea how many times I've been bitten over the years with mocking and field injection where if a field (that is field injected) is added later on, then the tests break, whereas with constructor injection (& setting the fields as private final) your IDE complains immediately about it, which is a good thing.

Additionally, having fields as package private just feels wrong to me. Fields should be private, and private final is even better.

Just my own $0.02 but I just don't like field injection and always prefer constructor injection.

[Chealer]

For context, sonar-java 7.27.0.33463 added S6813 "Field dependency injection should be avoided". I know nothing about SonarQube, but from my understanding upgrading from SonarQube 9.9 to 10.6 will add that check and therefore cause those using field injection to start getting such warnings. SonarQube 10 seems scheduled to become the "LTA" version this year.

There will likely be numerous Quarkus users wondering how to deal with that soon, since Quarkus's documentation implicitly suggests field injection, as can be seen in this prominent example. Moreover, SonarQube's check has defaultSeverity Major and a impact defined as MEDIUM. Our own projects have roughly a thousand affected fields. One user was already wondering about this on Stack Overflow nearly a year ago.

It would therefore be quite interesting for Quarkus to take an official stance on this matter, and perhaps provide an opinion about how severe this issue truly is in the context of Quarkus. If the impact is lower as @Ladicek suggests, perhaps SonarQube's check could take that into account. And if the impact truly is medium or higher, then surely Quarkus's documentation should not feature such examples without warnings.

Thank you very much for bringing this up @BrianH85 and all others for your contribution

[edeandrea]

HI @Chealer! We're actually in the process of debating this! We're working with the Sonarsource team directly on some proposals.

Feel free to follow/chime in: #43179 (comment)

[Chealer]

I had completely missed that; thank you so much @edeandrea, that's amazing!
@Ladicek , @manovotn: You might want to chime in there.

[mkouba]

I have to admit that I strongly disagree with such warnings - see my comment here: #43179 (reply in thread).