Spring @Autowired fields - which access modifier, private or package-private?

Question

Let's say that we use the @Autowired annotation over various fields in a class, and that we didn't write setters or constructors that can also set the fields.

Question - what should the access modifier be, private or package-private (i.e. none) ?

For example:

public class MyClass {
    @Autowired
    private MyService myService;
}

vs

public class MyClass {
    @Autowired
    MyService myService;
}

In the first case (private fields) Spring uses reflection to wire up the field, even if it doesn't have a setter.

The second case (package-private fields) allows us to be able to access those fields (for example, to set up mocks) if we need to extend the class for testing purposes.

So both cases work fine, but which is more recommended, particularly with regards to testing?

Sprinkle

If your testing framework can inject mocks in private fields, then go private. Otherwise, go package-protected. It doesn't matter much anyway, unless you fear that junior developers access package-protected fields from another class of the same package. – Implacental

Answer 1

The first case also allows you to inject mocks depending on the framework. For example using the @InjectMocks annotation of Mockito. You also have ReflectionTestUtils.setField in Spring test, ...

I'm personally not too fond of modifying classes too much for testing purposes, so I would go for the first case. But at the end of the day this mostly depends on your preferred test framework.

Iranian

Thanks Simon. I ended up using ReflectionTestUtils.setField() and making minimal changes to the class. The situation was that I wanted to disable a "live" https call in a class under test. So I put the https call into a one-line protected method, and overrode the method to do nothing when I tested it (thus the problem with private fields came up). Perhaps you can kindly also give me some advice on whether I was taking a valid approach here with disabling the https call? – Sprinkle

That's hard to comment on without seeing more of the code, but overriding a method so it does nothing in a test does not sound like the best solution to me. It's probably possible to use a mock object to prevent the call. – Iranian

Why not putting the responsibility of the HTTPS call to an external, injected component, that can be mocked in tests? You could also use a Spy or partial Mockito mock to "override" this method without the need of creating an explicit subclass. – Implacental

Yes, good idea @JB. The code actually calls a static method (HttpsUtil.doHttpsCall(...)) - I can definitely see the wisdom of using a component here however I wanted to make minimal changes to the code under test. – Sprinkle

Answer 2

So both cases work fine, but which is more recommended, particularly with regards to testing?

I think the properties should be private:

@Autowired
private MyService myService;

As it is always good to have getter methods to provide access to the properties instead of allowing other classes to have direct access to them.

And for testing purposes, injection of mocks of private properties will work the same way as that of package-private properties.

For example, with Mockito, you can inject a mock of private MyService into MyClass as this:

public class MyClassTest {
    @Mock
    MyService service;
    
    @InjectMocks
    MyClass serv = new MyClass();
    
    @Before
    public void init() {
        MockitoAnnotations.initMocks(this);
    }
}

Casemaker

Hi and thanks for the tip about @InjectMocks. Will this also work with spring wiring from xml test application contexts and @Configuration classes? The problem I have is that the mock of MyService (the same mock, not a different one) must also be available in a different class that is autowired by spring. – Sprinkle

you are referring to unit testing or integration testing? whenever you are using mock of MyService, you need to do method stubbing on that to behave the way you want it to behave. So two mocks of MyService with same method stubbing will behave like the same mock being used in two different test classes. – Casemaker

Well it's unit testing but we are using the "processor" design pattern, so there are a bunch of "processor" classes with one method (you guessed it - process()), and so next to nothing in really will get tested unless all these processors are wired up. Anyway, thank you for your input and I'm sorry that there were 2 good answers here, I chose to accept Simon's. – Sprinkle

Answer 3

I generally prefer having the field private and using setter injection:

public class MyClass {

    private MyService myService;

    @Autowired
    public void setMyService(MyService myService) {
        this.myService = myService;
    }
}   

allowing the service to be @Autowired, but set with a mocked instance for unit testing.

Arctogaea

I can see that this is perhaps better for testing, but it's also more verbose and in the case I am working on I wanted to make minimal code changes to facilitate testing. – Sprinkle

Answer 4

I would generally NOT use @Autowired for private fields or methods. @Autowired means, somebody from outside will set this field. "Private" on the other hand means nobody except this class is allowed to use it.

Mixing @Autowired and private can theoretically cause problems, if the JIT compiler somehow optimizes this code. It can be a Java Memory Model concurrency related problem, which will be production-only and impossible-to-reproduce.

I would make the Autowired fields at least package visible. As free bonus it will allow to write unit tests without tricks and workarounds.

UPDATE: Additionally I would declare such fields as volatile to avoid Java Memory Model related visibility conflicts. Spring developers do some tricks to make autowired fields work without explicitly synchronizing the access, but I am not sure these tricks are working smoothly in any JVM on any hardware.

Elisaelisabet

Answer 5

I would rather use private on @Autowired fields, for a few reasons:

• I use these fields for dependency injection, generally in service classes, to use other service classes. In that case I want to keep those fields to the current class.
• Also, extending this class can lead to different logic, so maybe another implementation of the @Autowired fields is needed, hence the private instead of package-private.
• Furthermore, when refactoring, it helps to see when such a field is not used anymore, as package-private fields don't show a warning when unused (assuming your IDE is Eclipse - I actually don't know for other IDEs).

Adela