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.
27 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.
Hi
I have a question about circular dependency…
I have a class Character who is responsible for many and many things : appearance (face, hair…), status (lifepoints…), stats (attack power, defense power…) and i’m stuck !
Look at an example :
class Character {
private CharAppearance ca;
private CharStatus status;
private CharStats stats;
public Character(CharAppearance _ca, CharStatus _status, CharStats _stats) {
//
}
}
Example of CharStatus :
class CharStatus {
private int lifePoints;
private Character owner;
public CharStatus(Character _owner, int _lifePoint) // Here.. circular dependency !
}
How to solve that issue ? Should I remove the owner of CharStatus ?
Thanks a lot
Thomas.
Hello again
Well, I think I have found something …
class Character {
private final Client client;
private final CharStatus status;
// other fields…
public Character(Client _client, CharStatus _status);
}
public CharStatus {
private final Client client;
private int lifePoints;
public CharStatus(Client _client, int _lifePoint);
public void fireStatusUpdated(String _property, int _old, int _new); // Using Observer/Observable pattern
}
And finally, class “C” :
class Client {
public void onStatusUpdate(Client _client, String _property, int _old, int _new);
}
The issue was I wanted my collaborator class to send an update to my “super class” Character. Then, my Character send an update to the Client, then Client send data throw network…
In fact, collaborator classes have to send an update directely to the client…
Circular dependency avoided
Sorry for posting !
Thomas.
Ps : I love your job, I think DI is the key for keeping a clean code.
Ps2 : Sorry for ma poor English =) I’m French, and French reputation is to be bad at English ^^
@Thomas,
Glad you figured it out before I get to respond.
@Misko,
Well, i was trying to solve that problem since a week
I’m waiting for new post/presentation about Oriented Object Concept !
Good luck, and thank you !
nice post – came through SO.
I’ve been playing with Apache wicket and ran into this problem. Here’s the scenario:
PageBorder – a border component that wraps around a BasePage and does stuff like setting title, meta etc.
class PageBorder extends Border {
public PageBorder (id, BasePage page){
this.page = page;
add (“title”, new Label(“title”, page.getTitle());
}
}
At the same time, since the BasePage contains the PageBorder, the BasePage needs to know about the border (create the border passing this as a ctor param)
public abstract class BasePage extends Page {
PageBorder border;
public add (Component c) {
if (border == null) {
border = getBorder();
// subclass must create a border passing themselves in
add (“border”, border);
}
border.add(c);
}
}
In this case, in the BasePage class, I’ve put a protected method – getBorder() – so that the implementing page can decide which border to create.
While this set up works, somehow this seems smelly.
OTOH, if I look at wicket itself, seems like this mutual knowledge is rampant (Page class has a getComponent) and Component classes have a getPage() – however, the coupling is lower.
Any suggestions?
Nice post, misko. I find that this is generally true, though when boot-strapping one system within another, there’s often a circular dependency at the border. For instance, if the engine creates a context in which to load scripts, the root script might need a reference back to the context in order to load more scripts.
Say you have two classes, Teacher and Student.
Each student can have multiple teachers & each teacher can have multiple students. The student would need to reference the teacher & the teacher would need to reference the student. so, you would have
t -> s
t <- s
How would you work around this?
Hi, Spencer.
The relation is not like this. Actually you have this:
Teacher{
List teachedCourses
}
Course {
Teacher teacher
List attendingStudents
}
Student {
List myCourses
}
So t -> s and s -> t are derived relations. You wouldn’t pass S to T nor T to S in the constructor.
In the next step I would dispute bidirectional references above…
Oleg
How about something like listener and provider interfaces?
I could have a class that has a port connection and is able to provide to a single listener. However the thing listening also wants to be able to send things out on the port so needs a reference to the port (whether directly or via an interface)
interface Listener
{
void heardSomething(String thing);
}
class Port
{
Port(Listener listener);
void write(String stuff);
}
class ClassA implements Listener
{
ClassA(Port writeTo);
}
Now when it comes to creating this, I need to be able to pass ClassA to Port (as an interface) and Port to ClassA as follows:
main
{
ClassA a = new ClassA(Port…?)
Port p = new Port(a);
}
Now I know that this could be loosened up by allowing more than one listener and rather than passing in the constructor using an AddListener approach, but I want to see how you would work this with the 3rd class approach.
Matt
Constructor Injection vs. Setter Injection | Abhijit Gaikwad // May 30, 2012 at 7:36 pm
[...] 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 [...]
I’m a bit confused by “list all of the methods in your class A used by class A” … wouldn’t they all be used?
Cyclic Dependency Injection and Making a Choice | Tim's Blog // Jul 26, 2012 at 3:01 pm
[...] according to Miško Hevery there is a solution to circular dependency. You can expose a hidden 3rd class that takes both the AclModel and the CustomerModel and decouples [...]
I like the solution you’ve posted. It solves the issue with using constructor injection for circularly dependent classes.
Currently, I’m thinking of how to break the circular dependency between the View and Presenter in MVP-passive view pattern.
Here’s a simplified code.
class Presenter {
final Model model;
public IView view;
Presenter(Model model) {
this.model = model
}
}
interface IView {
}
class View implements IView {
final Presenter presenter;
View(Presenter presenter) {
this.presenter = presenter;
this.view = this;
}
}
Constructor Injection vs. Setter Injection | TechInterviewPrep // Mar 9, 2013 at 4:00 pm
[...] 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 [...]
Leave a Comment