Adam Bien's Weblog

Wednesday Jun 15, 2011

@Inject With Package-Private Fields Only, Or Harmful Accessors

I do not use private, or any other modifier for all injected (@Inject, @EJB, @PersistenceContext) fields:


@Startup
@Singleton
//...
public class Hits {

    @Inject
    XRayLogger LOG;
    @EJB
    PersistentHitStore hitStore;
    @EJB
    PersistentRefererStore refererStore;
}

This simplifies mocking considerably: your unit tests (hopefully) reside in the same package as the production and can directly access all the injected fields:

class HitsTest extends JUnitSuite with MockitoSugar with ShouldMatchersForJUnit {

  var cut: Hits = _

  @Before
  def setUp: Unit = {
    cut = new Hits()
    cut.LOG = new DevNullLogger()
    cut.hitStore = mock[PersistentHitStore];
    cut.monitoring = mock[Event[Diagnostics]];
    cut.refererStore = mock[PersistentRefererStore]
}

[from x-ray]
Package-private fields do not belong to the EJB 3.1 no-interface view and are not visible to other bean instances.

An alternative approach would be to make a field private and then expose it via a public setter:


private XRayLogger LOG;

@Inject
public void setLog(XrayLogger log){
	this.LOG = log;
}

Although the example above implies encapsulation, it actually breaks it. A public method belongs per convention to a non-interface view of an EJB 3.1 and could be regularly invoked from the injected instance. You could of course add an explicit interface to hide the setter, but this would be the first step towards overengineering.

[All x-ray unit tests are accessing the package-private fields directly. See aslo page 110 "Injection and Infrastructure Testing with Aliens" in Real World Java EE Night Hacks--Dissecting the Business Tier.]


Special Event: Java 8 with Java EE 7: "More Power with Less Code", 13th October, 2014

A book about rethinking Java EE Patterns

Comments:

U can also use easygloss to mockup your private injected fields without declaring a setter.

<dependency>
<groupId>com.stvconsultants</groupId>
<artifactId>easygloss</artifactId>
<version>1.3</version>
<type>jar</type>
<scope>test</scope>
</dependency>

ciao

Posted by Felix on June 16, 2011 at 11:54 AM CEST #

@Felix,

sure! But why? :-)

Package-private is just simpler and concise...

thanks for your feedback!,

adam

Posted by Adam Bien on June 16, 2011 at 12:50 PM CEST #

Nice idea, breaking with another premature common sense :-)

Also nice because without modifiers its even less code. I would even prefer to have the @inject in the same line. Unfortunately for some IDE formatters (e.g. Eclipse) its not possible to configure this only for properties...

Thx for the post!
Jan

Posted by Jan Wiemer on June 16, 2011 at 03:48 PM CEST #

@Jan,

+1.

I actually started with this approach, because I do not like getters/setters as general "pattern":

http://www.adam-bien.com/roller/abien/entry/encapsulation_violation_with_getters_and

thanks & enjoy deleting code :-)

adam

Posted by Adam Bien on June 16, 2011 at 04:26 PM CEST #

Hi Adam,

nice post. I don't know if you ever heard of the book "Growing Object Oriented Systems Guided by Tests" by Steven Freeman and Nat Pryce. In this book they have a very good sample application where they put all their tests (unit, integration and acceptance) in a separate package (they put a 'test.' in front of the actual packages) to ensure that only the public API of the class under test will be checked.

Would it not be better to make the dependencies of the class visible through a constructor? This ensures encapsulation by not creating any setter methods for the properties and you can make them private (final) again.

What do you think?

Posted by Claus Polanka on June 18, 2011 at 10:38 PM CEST #

@Claus,

I already don't like the idea of making the attributes package-private just for the sake of testing. It is,however, the most pragmatic compromise I could find - and it works great so far (I'm doing that since 2006 - the advent of Java EE 5).

Constructors are bit problematic - you will have to provide multiple constructors. ...And this is the first step towards bloat :-).

thanks for the pointer - I'm going to order the book :-)

adam

Posted by Adam Bien on June 19, 2011 at 12:54 PM CEST #

Hi Adam,

good to hear. The book is a "mind-changer" in many ways. I don't understand why there have to be many constructors? One constructor where you pass in all the the _real_ dependencies of that class. Those you also would declare private final and only provide getter (if necessary at all). If you have too many of those, maybe your class is doing too much (SRP)? In the tests you mock those objects (if necessary) which you will need for the test. The others you can stub out (e.g. pass null).

So, at most you have two constructors: the default one (if a 3rd party framework e.g. Hibernate needs it) and the one which declares the real dependencies of the object.

What do you think?

Cheers

Posted by Claus Polanka on June 19, 2011 at 09:17 PM CEST #

@Claus,

at least two constructors: default and custom. A single, custom, constructor is sufficient in case you are going to pass all dependencies at once.

But I would prefer the solution: http://www.adam-bien.com/roller/abien/entry/inject_with_package_private_fields#comment-1308210841555

instead of constructors...

thanks!,

adam

Posted by Adam Bien on June 19, 2011 at 10:12 PM CEST #

Hi again,

ok, so I don't actually get it why you would have more than one custom constructor. Am I completely wrong if I think that a constructor should declare the _real_ dependencies of an object? If I have more than one custom constructor that sounds a lot of that this class represents more than one concept. Other dependencies which could change over the lifetime of an object should have default values and setters anyway. I don't want to sound to dogmatic but maybe after reading that mentioned book you will also see where I want to point at with my comments ;-)

But of course your way is an absolutely valid one and I will consider it in my next projects where I maybe have a concept which should be only package private where I can only write tests within this package for that object.

Cheers

Posted by Claus Polanka on June 20, 2011 at 01:54 AM CEST #

Hi all,

interesting discussion!

I think 'dependencies' in general is a quite fuzzy term. From which other objects is an object dependent? From all objects referred in properties? Only from the 'important' ones? Or even from all referenced objects (even those passed as parameters)?

IMO history has shown that wiring objects together (using constructors or setters) is often quite complex. In some cases you have to think more about the proper wiring than about the business logic. And this is where dependency injection (DI) helps.

So for business logic DI helps getting rid of the constructors. Now I would prefer not to reintroduce the constructors (anyway many code lines, most often at the top of the class) only for testing...

What did you think?

Regards,
Jan

Posted by Jan Wiemer on June 20, 2011 at 09:39 AM CEST #

Hi Jan,

my definition of an object's dependencies is from the above mentioned book: "Dependencies: Services that the object requires from its peers so it can perform its responsibilities.
The object cannot function without these services. It should not be
possible to create the object without them."

Thus those dependencies must be passed through the constructor. Normally they are "private final" to avoid mutable state.

Please be careful with how you are using the term DI. Only property- and setter- DI gets rid of constructors which in my understanding is not a good thing. If some body calls an object and there is no constructor showing the necessary dependencies, how could you know that the object is not in an invalid state after you've created it.

That's the problem with setter and property injection. And don't forget... you don't need a framework for dependency injection!!!!

Cheers

Posted by Claus Polanka on June 20, 2011 at 09:11 PM CEST #

@Claus

Ah, I think I got your point now :-) The idea is to provide a constructor to document the natural dependencies. The good thing is that we can use DI for the constructor arguments as well. Otherwise we would come out with the wiring problem again.

Interesting idea. I have to see how it feels in the code!

Thx!
Jan

Posted by Jan Wiemer on June 21, 2011 at 10:08 AM CEST #

Hi!

Adam, and what about CDI alternatives?
If package-private access modifier is simplifying the testing, what do you think about using private access modifier with @Inject and just modify mocking objects through @Alternative annotation?

Thanks for your blog,

Cheers

Posted by PedroKowalski on July 12, 2011 at 10:06 AM CEST #

@Pedro,

"Adam, and what about CDI alternatives?"

Alternatives - do you know any? :-)

"If package-private access modifier is simplifying the testing, what do you think about using private access modifier with @Inject and just modify mocking objects through @Alternative annotation?"

IMHO: this is too intrusive. Package-private is just the simplest possible thing so far.

thanks for reading my blog!,

adam

Posted by Adam Bien on July 13, 2011 at 12:55 AM CEST #

@Adam

"Alternatives - do you know any? :-)"

Nice one :-)
Of course by saying 'CDI alternatives' I meant the CDI's @Alternative annotation :-)

"IMHO: this is too intrusive. Package-private is just the simplest possible thing so far."

Yeah, I agree that it seems the simplest possible solution, but I took this idea from the Weld documentation: http://docs.jboss.org/weld/reference/1.0.0/en-US/html/injection.html#alternatives

BTW [OT]: Your latest book is just great! Written in a simple and clear way, showing a variety of tools and techniques.

It follows my favourite 'hey, watch this - it's easy' way instead of 'hey, watch how smart I am'.
Hats off!

Cheers,
Pedro

Posted by PedroKowalski on July 14, 2011 at 09:56 AM CEST #

The way I learned it, white box unit tests go in the same package and black box unit tests go outside of the package.

If I want to do an integration test, and I have bean1 which calls bean2, and I want to mock a call from bean2 to bean3, then I can't use the method described above because my integration test for bean1 might be in the same package as bean1, but it won't be in the same package as bean3 (assuming the beans are all in different packages). I can see three options: a) use reflection, b) have a public setter and c) provide a constructor for setting dependencies. All have pros and cons.

The constructor at least can't be used by an cowboy to set the value of an injected instance to something else.

Reflection doesn't need to have a breakable dependency on the field name. It could simply walk through all fields annotated with @Inject / @EJB / whatever, and look at the mocks you provided to your reflection utility, much like the container itself does.

Posted by Ant on June 18, 2012 at 07:19 PM CEST #

Post a Comment:
  • HTML Syntax: NOT allowed
realworldpatterns.com
...the last 150 posts
...the last 10 comments
Links
License