Is it bad practice to use dependency injection in factory classes?

Question

Is it bad practice to use dependency injection in factory classes? Should I let the users of my framework take care of dependency injection? Should I use approach A or approach B?

SomeUserClass

package com.impl;

@Service
public class SomeUserClass {

    @Autowired
    private SMSActionFactoryService actionFactoryService:

    @Autowired
    private PropertyManager properties;

    public void doStuff(){
        // approach A
        SMSAction action = actionFactoryService.createAction("hello");

        // approach B
        action = SMSActionFactory.createAction(properties, "hello");

        // the user should never call Action::doAction. 
        // It gets called by the framework on a condition.
        scheduler.addAction(State.ERROR, action)
    }

}

SMSAction

package com.framework;

public class SMSAction extends Action {

    public SMSAction(PropertyManager properties, String message){

    }

    public void doAction(){
    }

}

SMSActionFactoryService

package com.framework;

@Service
public class SMSActionFactoryService {

    @Autowired
    private PropertyManager properties;

    public SMSActionFactory createAction(String message) {
        return new SMSActionFactoryService(properties, message);
    }
}

SMSActionFactory

package com.framework;

public class SMSActionFactory {

    public static SMSActionFactory createAction(PropertyManager properties, String message) {
        return new SMSActionFactory(properties, message);
    }
}

Approach A makes unit testing easy. Approach B makes unit testing a pain. Avoid pain! (And follow TDD or at least write unit tests soon after to quickly discover code smells.) — Andrew S, Jan 22 '18 at 14:59

score 0 · Answer 1 · answered Jan 22 '18 at 15:44

I think you have a context problem, so the answer depends on the context. But I'll give some of my experience, and not a formal (and irrefutable) answer. Based on the title of the answer (practices) I'll give you what I call good practices tips that helped me a lot when I started Spring development.

First of all, let's think about the Dependency Injection you have. You're wiring a field, and we know that the Spring team used to suggest us to use constructor based injection (and assertions for all mandatory dependency) as you can see here. Well, I know it was a problem with the tests framework that couldn't wire the dependencies in an easy way, but now they can. But there's another advantage using this pattern, you can make your bean field final. Another advantage is that you prevent circular dependencies, like X depends on Y and Y depends on X and so on. So, as the first tip, I would suggest you to use something like:

private final SMSActionFactoryService actionFactoryService:
private final PropertyManager properties;

@Autowired
public SomeUserClass(SMSActionFactoryService actionFactoryService,
                     PropertyManager properties) {
     Assert.notNull(actionFactoryService, "The actionFactoryService bean is null, you should provide the bean to run this application");
     Assert.notNull(properties, "The properties bean is null, you should provide the bean to run this application");

     this.actionFactoryService = actionFactoryService;
     this.properties = properties;
}

This way you prevent any other code part to change the field value. As you can see in Spring autowiring setter/constructor PROs and CONs this is a preference subject.

Now, for the second tip, I wouldn't use @Service for a factory, not even @Component because factories needs to be open for extension and close for modification. You're going to understand better if take a look here.

That said friend, I suggest you to embrace approach B.

The SomeUserClass is not part of the framework, it is just an implementation example. There are different ActionFactory classes, so I can't just pass a Factory to the user. The user has to learn about the different ActionFactory classes and inject them itself. — Paul White, Jan 22 '18 at 16:02
Well, I could add an ActionFactoryGodClass which can create all different Actions, but I don't think thats a good approach. — Paul White, Jan 22 '18 at 16:07

Is it bad practice to use dependency injection in factory classes?

1 Answers1