I have situation like this. I cannot see any errors but I am not getting my results.
@ApplicationScoped
public class A {
private B b;
@Inject
public A(B b) {
this.b = b;
}
}
@Singleton
public class B {
private A a;
@Inject
public B(A a) {
this.a = a;
}
}
Is this type of dependency injection is wrong?
Can any one help me with this.
I'd avoid this circular dependency, there is a few reasons to do that.
Comment on this article
A messy constructor is a sign. It warns me that my class is becoming a monolith which is a jack of all trades and a master of none. In other words, a messy constructor is actually a good thing. If I feel that the constructor of a class is too messy, I know that it is time to do something about it.
And this one
You’ll find cases where a class A needs an instance of B and B needs an instance of A. This is a typical case of a circular dependency and is obviously bad. In my experience the solution is either to make B a part of A when the two are so strongly dependent that they really should be one class. More often though there is at least one more class C hiding in there so that B doesn’t need A but only C.
As Oliver Gerke commented:
Especially constructor injection actually prevents you from introducing cyclic dependencies. If you do introduce them you essentially make the two parties one because you cannot really change the one without risking to break the other, which in every case is a design smell.
Here is a small example of what I might do.
public class A {
private B b;
@Autowired
public A(B b) {
this.b = b;
}
public void doSomeWork() {
// WORK
}
public void doSomeWorkWithB() {
b.doSomeWork();
}
}
public class B {
private A a;
@Autowired
public B(A a) {
this.a = a;
}
public void doSomeWork() {
// WORK
}
public void doSomeWorkWithA() {
a.doSomeWork();
}
}
After refactoring it might look like this.
public class A {
private C c;
@Autowired
public A(C c) {
this.c = c;
}
public void doSomeWork() {
// WORK
}
public void doSomeWorkWithC() {
c.doSomeWorkThatWasOnA();
}
}
public class B {
private C c;
@Autowired
public B(C c) {
this.c = c;
}
public void doSomeWork() {
// WORK
}
public void doSomeWorkWithC() {
c.doSomeWorkThatWasOnB();
}
}
public class C {
public void doSomeWorkThatWasOnB() {
// WORK
}
public void doSomeWorkThatWasOnA() {
// WORK
}
}
Quoting from Section 5 of the CDI Specification 1.2:
The container is required to support circularities in the bean dependency graph where at least one bean participating in every circular chain of dependencies has a normal scope, as defined in Normal scopes and pseudo-scopes. The container is not required to support circular chains of dependencies where every bean participating in the chain has a pseudo-scope.
ApplicationScoped is a normal scope, so this cycle should work.
In your sample, class A cannot be proxied since it's missing a zero-argument constructor. Adding this constructor (which may have protected or package visibility), your sample deploys without problems.
You can also use Setter based Dependency Injection to resolve this issue.
There is definitely a solution to this. Let me quote myself:
The right solution is to inject javax.enterprise.inject.Instance, where T is type of the class to be injected. Since the type is directly Foo, calling get() method on an object typed as Instance is guaranteed to inject the right object all the time. This approach works very well, because the instance is obtained dynamically from the container by the implementation itself and only as needed. Therefore, the responsibility of dependency retrieval is left to your code - your code is responsible not to make an endless loop.
@Named
public class Foo implements Fooable{
@Inject
private Instance<Foo> foo;
public void executeFirst(){
foo.get().executeSecond();
}
@Transactional
public void executeSecond(){
//do something
}
}