Are circular class dependencies bad from a coding style point of view?

Question

Are circular class dependencies bad from a coding style point of view?

Example:

In a database application we have two classes, one encapsulating information about a single database (DBInfo) and one class which can create a database connection. (ConnFactory)

DBInfo has a getConnection method which uses ConnFactoryto create a connection. But ConnFactory itself needs a DBInfo object to do so.

Like this: (Any coding styles disregarded for the sake of readability)

class DBInfo {
    String name;
    String connectionUrl;

    Connection getConnection() {
        return ConnFactory.getConnection(this);
    } 
}


class ConnFactory {
    Connection getConnection(DBInfo toWhat) {
        return new Connection(toWhat.connectionUrl);
    }
}

My co-workers argue that this is bad practice and it would be better if there were only one direction of dependencies and no circular ones like here.

Is this bad practice, an anti-pattern or a code smell? Are there any drawbacks?

Answer 1

All I know is that circular dependencies can become a bit of a problem when you start using a Dependency Injection Framework such as Structure Map. Most of these frameworks have trouble handling circular dependencies, sometimes resulting in a stack overflow exception (pardon the pun :-)) Therefore I tend to try to steer clear of it unless absolutely necessary and cannot be avoided.

Answer 2

This code only works if ConnFactory.getConnection() is static. A better solution would be to make getConnection() an instance method of ConnFactory. Then your DBInfo can take a ConnFactory as an argument (possibly in a constructor, if you have an overloaded constructor). However, I think the use of the static method for this case is more of a bad practice than the circular reference.

If you were to go this route, though, I would also make an interface IConnFactory that DBInfo would interact with and that ConnFactory would implement. Then there is no circular reference -- both DBInfo and ConnFactory would depend on IConnFactory, which would depend on neither.

Answer 3

Yes, generally speaking circular dependencies are bad, though not always evil. Problems with circular dependencies include tight coupling, mutually dependent modules and generally domino effect, when changes in one module propagate to other modules.

That said, your code is violating Single Responsibility Principle in that DBInfo not only stores information about the database, but is also responsible for obtaining Connection objects. Remove that particular piece of functionality to a separate class and everything will be just fine.

Answer 4

In general, I would call circular dependencies a Code Smell. Note that the term 'Code Smell' mainly indicates that 'here is a piece of code that requires special attention, and is likely to benefit from redesign.'

In most cases I would strongly consider a design where a circular dependency is not necessary, but in rare cases it may be okay.

In your example, the ConnFactory seems redundant, but that may be because your example has been trimmed down. It seems to me, however, that the Connection creation logic would be better if it was moved to the DBInfo class. When you already have a class that contains data about a database, it seems only natural to make it responsible for creating a connection to that database.

Answer 5

Circular dependencies are bad because:

• two dependencies is more than one
• you can't incrementally test (without mocking one of them, which would be silly for small, closely coupled things).

You can do all the stuff with interfaces to break the circular dependency if required, but the straightforward minimal solution is to just make DBInfo a nested class of ConnFactory. A unit that references itself isn't circular.

Answer 6

Not necessarily

I don't think circular dependencies at the class granularity level are bad. I don't see a problem if two, three or perhaps four classes are mutually dependent. (I am not saying this is something you want, but it can be ok in some circumstances).

It is a problem if you have mutual dependency at the package or module level, for all the reasons mentioned above and below.

Answer 7

What about a bi-directional one-to-many relationship that is such a common case in any application using an ORM layer? Isn't this a case of a circular dependency?

Is it bad/code-smell ?