Why Not Private Visibility For Injected Fields [Screencast] : Adam Bien's Weblog

Hi Adam,

Why don't you set the injected fields private and use reflection in you unit test to set the fields?

Greetings from Switzerland
Simon

Posted by Simon Martinelli on June 16, 2012 at 09:10 PM CEST #

hi, nice video!
What if you keep it private and only provide the setter?

Posted by Marcos on June 16, 2012 at 09:49 PM CEST #

@Simon,

"Why don't you set the injected fields private and use reflection in you unit test to set the fields?"

This would work of course, but does the use of reflection in unit test improve the maintainability or productivity in any way?

cheers - and thanks as always for the comments,

adam

Posted by Adam Bien on June 17, 2012 at 09:11 AM CEST #

@Marcus,

"What if you keep it private and only provide the setter?"

Would a setter improve anything? :-)

thanks!,

adam

Posted by Adam Bien on June 17, 2012 at 09:12 AM CEST #

I don't think the issue is about breaking encapsulation. Field injection violates a core principle of DI: exposing dependencies. Your class needs the dependency because it would throw an exception in case the field is null. So you should not allow the class being set up in an invalid state. This is exactly what field injection allows. You essentially have to know you have to configure the dependency field (as you do in the unit test) which requires knowledge about the implementation and some kind of hack to access and manipulate the state. I wouldn't actually want to rely on some arbitrary container to prevent some arbitrary access in some arbitrary context.

If you used constructor injection instead the necessary dependency was exposed through the API and the IDE code completion would neatly throw that dependency into the developers face. Beyond that you could add an assertion to prevent null being handed as dependency (which in turn would set up the object invalidly).

Jens Schauder had an interesting blog post discussing this topic recently: http://blog.schauderhaft.de/2012/06/05/repeat-after-me-setter-injection-is-a-symptom-of-design-problems/#comments

Posted by Oliver Gierke on June 18, 2012 at 06:35 PM CEST #

I am using still private Fields and @Getter and @Setter annotations of project lombok. Thats more convenient and doesn't break the rules.

Posted by Christian Sterzl on June 19, 2012 at 11:52 AM CEST #

Having package protected fields in CDI breaks the principle of least astonishment.

You provide a clearly formulated API to access the guts of an object and you might even show in a unit test that it works. But in the final runtime environment an exception is thrown.
An indication for that is that you had to show it in your screencast because you thought it's not obvious.

Keep up with your great work

Posted by Stefan Riesen on June 19, 2012 at 12:16 PM CEST #

IMO, classes broke encapsulation;
programming in interfaces don't.
you choose classes for speed.

screen-cast reveals what field's package visibility allows you to write code which is type-safe at compilation stage but fails type-safety in glass-fish runtime.
i do not yet understand what and why redefined class `Bondary` to have field `control` private.

Posted by ~arumad on June 19, 2012 at 02:05 PM CEST #

a big fat -1 from me

Please remember that field access won't work if proxies being used. That is for _every_ EJB and CDI bean accessed from 'external'. By lowering this barrier users will often accidentally access the proxy instead of the real underlying instance. And the proxy itself will contain no data at all.

Also, a unit test should only in *exceptional* cases access internal fields at all! A unit test is for testing _behaviour_ and not the implementation details.

LieGrue,
strub

PS: see you on Tuesday

Posted by struberg on June 19, 2012 at 06:58 PM CEST #

What app server are you using? I just tried this on Glassfish 3.1 and accessing the control instance simply returns null without any exceptions.

Posted by Anton on June 19, 2012 at 10:13 PM CEST #

Hi Oliver,

"Field injection violates a core principle of DI: exposing dependencies..."

It does, but it also saves time. So far I never saw someone trying to access fields directly in the production code. If someone would attempt to directly access the fields tools like sonarsource would find the issue before even deploying.

Without private I'm writing less code with no plumbing and have only theoretical drawbacks.

thanks for you comment!,

adam

Posted by Adam Bien on June 22, 2012 at 09:11 AM CEST #

@strub,

"...a big fat -1 from me

Please remember that field access won't work if proxies being used. That is for _every_ EJB and CDI bean accessed from 'external'. By lowering this barrier users will often accidentally access the proxy instead of the real underlying instance. And the proxy itself will contain no data at all.
"

I think your big -1 is actually a big +1. Your nicely summarized the screencast and my point :-)

thanks!

adam

Posted by Adam Bien on June 22, 2012 at 09:20 AM CEST #

@Anton,

"What app server are you using? I just tried this on Glassfish 3.1 and accessing the control instance simply returns null without any exceptions."

I'm using Glassfish 3.1.2. In CDI you will get a null, and willbe able to change the value of the injected field, but not the real reference. This is the actual worst case.

In any case: I never saw someone trying to access fields in Java EE code.

thanks!,

adam

Posted by Adam Bien on June 22, 2012 at 09:24 AM CEST #

@Arumad,

"IMO, classes broke encapsulation;
programming in interfaces don't.
you choose classes for speed."

I only use interfaces if:
- The implementation can be named after the responsibilities and without
"Impl" ending
- You are starting with several implementations
...and there are some obvious reasons with true benefits.

See also: http://www.adam-bien.com/roller/abien/entry/how_to_deal_with_interfaces

thanks for your opinion!,

adam

Posted by Adam Bien on June 22, 2012 at 09:26 AM CEST #

> "Field injection violates a core principle of DI: exposing dependencies..."
>
> It does, but it also saves time.

Okay, so why do you write tests then at all? Not writing them saves a *lot* of time. It's not about the fastest way to write software, it's about writing maintainable, understandable software. Too much boilerplate kills that, but hacky abbreviations do as well.

Posted by Oliver Gierke on June 28, 2012 at 10:04 PM CEST #

@Oliver,

"...Okay, so why do you write tests then at all? Not writing them saves a *lot* of time..."

Pragmatic tests *do* save time, because you can minimize errors upfront.

IMHO it is all about probability. It is very likely that business logic will contain errors, but it is rather unlikely, that someone will even attempt to access a package-private field in production.

It is all about probability: http://www.adam-bien.com/roller/abien/entry/a_good_architecture_is_all

thanks!,

adam

Posted by Adam Bien on July 01, 2012 at 02:37 PM CEST #

Why not just use JBoss Arquillian to do in container testing. I find that it makes for much better test as you are testing behaviour in a real environment. Arquillian means that it is no longer necessary to have to pervert the code just so you can demonstrate passing an unrealistic test.

Posted by Magnus on July 03, 2012 at 12:50 AM CEST #

@Magnus,

"Why not just use JBoss Arquillian to do in container testing."

Arquillian is an integration test tool. I'm talking here about unit test only.

"I find that it makes for much better test as you are testing behaviour in a real environment."

I prefer to perform system tests on a freshly installed server for "real environment tests".

"Arquillian means that it is no longer necessary to have to pervert the code just so you can demonstrate passing an unrealistic test."

For pure business logic test, I don't need arquillian. Just pure JUnit. For unit tests, as originally defined: http://en.wikipedia.org/wiki/Unit_testing, no application server or any other kind of container are needed.

thanks for your comment! And I really like arquillian!,

adam

Posted by Adam Bien on July 03, 2012 at 06:14 AM CEST #