I’m rewriting somebody else’s code at the minute, and I came across two classes which reference each other directly and call methods on each other. Like so (example in C#):
class A {
B otherClass;
public A() {
this.otherClass = new B(this);
}
// Other methods here call methods on B
}
class B {
A otherClass;
public B(A otherClass) {
this.otherClass = otherClass;
}
// Other methods here call methods on A
}
Reading through his code and trying to understand it, I’m having to move between the two classes as they call each other.
These classes are not independently reusable without using the other. I’m aware it’s considered best practice to avoid circular references (and it’s straight up not allowed between C# projects in Visual Studio), but is this, or could this be, in your experience any better than having a single – larger – class?
1
It’s usually not about the two classes being independent as logical modules, but their lifetimes being independent. An excellent example is if A is a LinkedList and B is a Node. Conceptually, they’re just one thing, a LinkedList, but B has to be separate so that it can be created and destroyed independently.
If that does not apply, you may wish to consider creating a single larger class.
People rarely split classes without a good reason. Where they have trouble is in the details of how. Before recombining the classes, try reassigning methods between the classes and consider introducing a third class.
If a method calls into another class, try moving the method to that other class. This is sort of like converting a push into a pull. It accomplishes the same task, just the other way around. This exercise can sometimes greatly reduce the coupling.
The other thing that often helps is introducing a third class. This can be in the layer above, containing instances of both objects and arbitrating between them, in the layer below, like a POD object to share data, or some kind of result object, similar to a database cursor. A lot of people neglect this approach because it feels like instead of two tightly coupled classes you now have three, but this pattern is often instrumental in making the dependencies only flow one way.
While is it probably not the best setup, there could be some advantages to a pair of tightly coupled classes over one large class.
While working with the Google Web Toolkit in Java, I have run across this pattern often as a part of the MVP pattern. Class A is the view and Class B is the presenter. The view has dependencies on the browser DOM, making it difficult to test, while the presenter contains all of the logic. By injecting a mocked version of the view into the presenter, the application becomes much more testable.
If the classes have not been split for testability via mocking, then I would argue that this split, while perhaps a step in the right direction, is not entirely useful. Sure, it may help to separate the code by what it does (like if class A was managing database connections while B was managing querying), but the code could be reengineered to not have the circular dependency.
Do A and B have clearly separated concerns? If the answer is yes, it is good when they are kept as two separated classes. And if you want to avoid a circular reference, for example, to create unit tests for testing A and B separately, define interfaces for A, B or both and don’t call methods of A in B directly, only by using the interface.
As you’ve described the classes, I think it’s fine. It’s not really a circular reference. A is dependant on an instance B & B cannot exist without an instance of A. You cannot create an instance of B with an A.
The only thing that I’d be looking at is if the functionality within each class is appropriate.
ie think of each as a black box. Could you create a new derived class from B and use that in A? Could you do that without knowing what A was doing? Could you do the same with A without knowing what B was doing?
Class A and B could both derive from other classes, which would not be possible with a single class, since C# does not have multiple inheritance.
Sometimes this kind of mutual dependency seems quite natural. Think of a car/driver relationship. It would not feel natural combine both into one class.
It would however be better not to make both classes directly dependent on each other, but let them know about each other through interfaces. This would enhance modularity and would allow you to provide another implementation for one class, while leaving the other untouched.
When you have two classes jumping back and forth between each other, it could mean you didn’t hit your abstractions correctly. Combing that code into one class seems like it would break Single Responsibility(if it’s not already broken) when keeping the SOLID principles in mind. When I am trying to work my way through refactoring legacy code, what seems to work best is getting each class under test. Your unit tests will help reveal what each class is trying to accomplish.