Generator-jhipster: Use constructor-based injection with Spring

[jdubois]

Sonar tells us we should use constructor-based injection with Spring.

I've always been a big fan of field-based injection, and for me there was a consensus in the Spring team to say that was the best choice (but now I see newer members of Pivotal would rather do constructor-based injection).

Anyway, I'm not going to recap all the pros/cons for each choice here, just that I'm changing my mind on this for a very specific new reason: Angular 2. I would like us to have something consistent in the back and front technologies, and with Angular 2 we are using constructor-based injection.

Can you vote on this comment with the GitHub emoticons? Vote up if you want constructor-based, and down if you want to keep field-based injection.

[deepu105]

To be frank I kind of like field injections so here is a blank vote

[sendilkumarn]

+1 especially because of angular2.

Personally don't like constructor but with angular2 it makes sense to have it

[gmarziou]

This would apply to @Service and @RestController annotated classes only, right?

[jdubois]

@gmarziou it would apply to all beans, and probably also for test classes (not tested yet, but should be OK).
And as Spring supports constructor-based injection without the @Inject annotation, that means I would remove the "javax.inject" package from the dependencies.

[gzsombor]

I prefer the field injections :

• less code to write (constructor not needed). Java - and javascript are a bit verbose, so I prefer to have less code
• I don't get why relevant if angular2 use constructor based injection on the server side. We don't use javascript on the serverside, neither we don't use java on the client side (maybe we can try it TeaVM or GWT ;-) )

[PierreBesson]

+1
This article managed to convince me that constructor injection is better : http://olivergierke.de/2013/11/why-field-injection-is-evil/
It also seems to be the recommended approach by the Spring team and also no @Inject annotation needed anymore and above all it should encourages to do more testing. So even though it's a big change I'm for it.

[xetys]

like @PierreBesson I was reading this article (which is linked by intelliJ, if you want to know, why field injection is evil)...and it sounds legit.

More on this, it would make the unit tests more readable, without reflection arcobatics for the rest controllers

[jdubois]

Yes that's the article I was refering to, and if we start to discuss this we'll go on forever. BTW I think this article is wrong, and I agree with @gzsombor - still I'm voting for constructor injection! So let's just vote and get done with this.

[deepu105]

Ok after reading what @gzsombor said I think we really dont have ro change just coz of ng2 as js and java are different and we have no obligation to keep implementations similar here. Also having annotations used to make it clear how its injected so I vote to keep as it is :)

[cbornet]

My position is to look at how they do it in Spring Boot and do the same.

[cbornet]

So I quickly checked the Spring Boot code and it's a clear win for field injection (in code, examples, tests, everywhere, ...). So unless there is a clear statement from Pivotal (and not just one of its members) that the new standard is constructor injection, I'm for keeping field injection.

[nkolosnjaji]

I'm for constructor based injection, doesn't brake unit tests and final can be used on field plus it will give more score on sonar

[notflorian]

I'm also for constructor injection, as Spring Boot started the migration in https://github.com/spring-projects/spring-boot/issues/5306 and https://github.com/spring-projects/spring-boot/issues/5543

[cbornet]

@notflorian good point: changing my vote !

[atomfrede]

Also Intellij already complain about field based injection (also known as
the Oliver gierke check ;) And +1 for constructor based injection mostly
because of better testability.

Am 28.11.2016 17:01 schrieb "Christophe Bornet" [e-mail]:

@notflorian https://github.com/notflorian good point: changing my vote !

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/jhipster/generator-jhipster/issues/4554#issuecomment-263310589,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAMaiefHWbswAOxabWc00aOUqZyRmJuiks5rCvrqgaJpZM4K81aR
.

[yhippa]

Sorry for piling on but it looks like in the Spring Framework Reference Documentation they advocate for constructor-based over setter-based injection:

The Spring team generally advocates constructor injection as it enables one to implement application components as immutable objects and to ensure that required dependencies are not null. Furthermore constructor-injected components are always returned to client (calling) code in a fully initialized state. As a side note, a large number of constructor arguments is a bad code smell, implying that the class likely has too many responsibilities and should be refactored to better address proper separation of concerns.

Setter injection should primarily only be used for optional dependencies that can be assigned reasonable default values within the class. Otherwise, not-null checks must be performed everywhere the code uses the dependency. One benefit of setter injection is that setter methods make objects of that class amenable to reconfiguration or re-injection later. Management through JMX MBeans is therefore a compelling use case for setter injection.

[jdubois]

Thanks for everyone voting -> so for me it's a YES for "constructor based" injection, and I'm going to close (and lock) this conversation as this is finished.

When I will code it, I will link the commit to this ticket.