by Miško Hevery
So you discovered dependency injection and GUICE and you are happily refactoring and writing new tests for you code until you come across this circular reference.
class A {
final B b;
A(B b){
this.b = b;
}
}
class B {
final A a;
B(){
this.a = new A(this);
}
}
+---------+ +---------+
| A |<-----| B |
| | | |
| |----->| |
+---------+ +---------+
Hm, dependency injection says that you need to ask for your dependencies and so the resulting code will be:
class A {
final B b;
A(B b){
this.b = b;
}
}
class B {
final A a;
B(A a){
this.a = a;
}
}
But now we have a problem, we can’t instantiate this (I know GUICE can through proxy, but it is not clean and it does not help us in tests). So the real problem in situation like this is mixing of concerns. One of the two objects is hiding another object C. Either A contains C or B contains C. To find out which one it is, list all of the methods in your class A used by class A, and all the methods in your class B used by class A. The shorter of the two lists is your hidden Class C.
+---------+ +---------+ | A |<-----| B | | | | | +-+ | | | | +->|C| | | |------+---->| | | | | | +-+ | +---------+ +---------+
Suppose B has the shorter list. We now extract all of the methods in B which are accessing the state of hidden C methods into a new object C like this:
+---------+
+---------+ | B |
| A |<-------------| |
| | | |
| | +---+ | |
| |--->| C |<----| |
| | +---+ +---------+
+---------+
class C {
C(){
}
}
class A {
final C c;
A(C c){
this.c = c;
}
}
class B {
final A a;
final C c;
B(A a, C c){
this.a = a;
this.c = c;
}
}
When you go through this exercise you will realize that the C was always an object in its own right but you have never thought about it that way, so the new code is actually better OO. From testing point of view you can now test each class in isolation.
13 responses so far ↓
Brilliant. I think I like this an *awful* lot, but I still don’t get one part. In your sentence:
“To find out which one it is, list all of the methods in your class A and class B and. The shorter of the two lists is your hidden Class C.”
There’s a strange sentence-terminating ‘and’ in there. I don’t understand what the two lists are. Could you state this again for clarity?
Thanks heaps!
I just found your blog, a fantastic read.
I just wanted to ask something.
Could you give a more specific example?
For example, lets say I have two classes:
Dog(Owner o)
+getOwner()
Owner(Dog d)
+getDog
.. what would you do in this case to break the circular dependency? What would your class C be?
@ Dennis,
Putting it like that, there is not enough information. What is it that Dog needs of owner? And What does owner need of Dog? You need to find the subset and extract it. Without knowing that information I can’t tell you the solution.
– Misko
Thanks for your response Misko. My example was a little contrived, sorry about that. I’m trying to use my imagination a little.
I’m just curious to see a more realistic example of how this technique could be used. I’m having trouble seeing what an example of class C might be.
Bravo! I have been trying to get people to understand this problem for years.
Constructor Injection vs. Setter Injection // Feb 19, 2009 at 11:19 am
[...] it a winner. Constructor-injection enforces the order of initialization and prevents circular dependencies. With setter-injection it is not clear in which order things need to be instantiated and when the [...]
“To find out which one it is, list all of the methods in your class A
and class B and. The shorter of the two lists is your hidden Class C.”I think this is supposed to be the following:“To find out which one it is, list all of the methods in your class A
used by class A, and all the methods in your class B used by class A. The shorter of the two lists is your hidden Class C.”
If the dev/process cost of creating object C is very high (in our case a large domain model backed by a DB schema), what would you consider the least harmful alternative until the model can be changed? Would you pull the logic into a factory? For example:
B createB() {
B b = new B();
A a = new A(b);
b.setA(a);
return b;
}
This would mean the factory contains logic beyond that of purely creating and composing objects.
@Nigel,
I am sorry, but I don’t understand your question. Can you explain it in more detail.
In general if I have legacy code and I creating an object is expensive, I first make sure that I can DI the object so that I can control the instantiation and hence where the cost is incurred. I would than try to pull out an interface so that I can write a mock.
Yes its legacy code
I can’t create an object C (yet) but still want to minimise the effect of the circular dependency. To do that I’ve pulled some of the setter logic up into the factory but this seems to contradict your answer to Q4 in ‘Flaw-constructor does real work’.
“The responsibility of the factory is to create the object graph and to do no work (All you should see is a whole lot of new keywords and passing around of references). The responsibility of the object graph is to do work, and to do no object instantiation.”
I’ve decoupled the objects enough to get them under test, but now in B’s case the back-reference is set within the object, and in A’s case its set by the factory.
It doesn’t ‘feel’ right but perhaps its just the cost I have to pay until I can create object C.
class A {
final B b;
A(){
}
setB(B b)
this.b = b;
}
}
class B {
final A a;
B(A a){
this.a = a;
}
}
B createB() {
A a = new A();
B b = new B(a); // Sets the backreference internally
a.setB(b); // Sets the backreference explicitly.
return b;
}
@Nigel,
That sounds right. I always have an uneasy feeling whenever I create circular dependency. Sounds like in your case there are some common things you need to pull out to C. I guess the question to ask is why do these two objects need to have circular dependency, on how can we rearrange the responsibility to break it. Without knowing details it is hard to make a suggestion.
This may be just what i have been after!.
Can you please help me understand it a little better.
Lets say i have two classes, a database service for writing to databases and a log service for writing to logs.
I want to pass a logger into the database service via a constructor so that it may the commands.
I also wish to pass a data service into the log service so that it may write the logs to a database.
Can you offer some method of refactoring this such that this is possible?
Secondly i think ive just realised that even if i could instantiate this mess then i will probably end up getting a stack fault when it executes a command, logs it, writes the message to the database, logs about that, logs it, writes a message etc etc…..Perhaps the refactoring will address this too?
public class LogService : ILogService
{
private IDataService _data;
public LogService(IDataService data)
{
_data = data;
}
#region ILogService Members
public void Write(string message)
{
_data.Execute(string.Format(”prcLogMessage {0}.”, message));
}
#endregion
public override string ToString()
{
return string.Format(”Complex Log Service with data provided by {0}.”, _data.ToString());
}
}
public class DataService : IDataService
{
private ILogService _log;
public DataService(ILogService log)
{
_log = log;
}
public void Execute(string command)
{
_log.Write(string.Format(”Starting execution: {0}.”, command));
// Execute command
_log.Write(string.Format(”Finished executing: {0}”, command));
}
public override string ToString()
{
return string.Format(”Complex Data Service with logging provided by {0}.”,_log.ToString());
}
}
@Peter,
here is how I would attack you problem.
class Database() implements DB;
class Logger(Database db);
class LoggingDatabase(Logger log, Database db) implements DB;
DB rawDatabase = new Database();
Logger log = new Logger(rowDatabase);
DB dbForTheRestoOfApplication = new LoggingDatabase(log, rowDatabase);
There you go. LoggingDatabase logs the commands as they go by and than delegate it on a class of the same interface. This is know as chain of responsibility and it is my favorite design pattern.
Leave a Comment