6

In a legacy project's service layer, there are tens of service classes and among them there is one service, UtilityService using 2 other services:

class UtilityService{
    private UserService userService;
    private ContactService contactService;
    //some methods using userService and contactService
}

This UtilityService is used in the controller level classes with other services. Some controller classes have depednencies on both UtilityService and UserService since some methods in UserService are needed in the controller class but they are not delegated to UtilityService:

class CustomerController {
    private UserService userService;
    private UtilityService utilityService;
    public List<Customer> getAvailableCustomers() {
      //here both userService and utilityService are in use
    }

}

IMO, the dependencies on both UtilityService and ContactService in one controller class are subject to circular references => Question: how to improve the code structure so that circular references can be easily avoided?

4
  • 8
    It's hard to say much more without seeing the full structure of your code, but my instant take is that if you can't find a better name for something than UtilityService (which really conveys no more information than ThingThing), you should consider whether that class should exist at all. Feb 19 at 18:29
  • 16
    Whether a circular dependency exists is a factual question and not a matter of opinion.
    – JimmyJames
    Feb 19 at 19:36
  • 6
    @Rui can you describe what you think a circular dependency is? (and why you think it's bad?)
    – Brondahl
    Feb 20 at 9:13
  • @Rui: if you like to describe what a circular dependency is according to your understanding, please edit your question, don't try to edit such an explanation into my answer.
    – Doc Brown
    Feb 20 at 20:39

2 Answers 2

36

The premise of this question is wrong - the example does not show a circular dependency.

The dependencies are

       CustomerController 
       |              |
       V              |
    UtilityService    |
       |     |        |
       |     V        V   
       |     UserService
       V
      ContactService

This is a directed acyclic graph - there are no cycles involved.

The fact the transitive dependency from CustomerController to UserService by UtilityService manifests itself explicitly as a direct dependency is not an issue, as long as it is not a design goal to make UtilityService a facade for accessing the service layer by the controller layer. The examples you gave show no indication for this.

For code, however, which really contains circular dependencies, have a look at this older SWE.SE question: How to solve circular dependency?

This answer shows how to resolve any kind of cyclic dependency by introducing interfaces. This answer shows how cyclic dependencies can be avoided in certain cases by creating smaller components with less responsibilities.

Note also resolving cyclic dependencies is not an end in itself, it is means to an end. A certain amount of cyclic dependencies (within one layer of a layered architecture, of course) can be acceptable, as long as testability and build times are not affected too much.

1
  • 1
    Years ago my codebase switched to interface files for each of the classes to resolve cyclic dependencies, and it took years for us to realize we did it wrong. The right thing is to create event listener interfaces. All the code depends on the event listener interfaces, and never on each other. With this, our tests are getting rapidly smaller and faster and less flaky, and the code is a lot cleaner. Feb 21 at 0:59
-8

One of the easiest ways is to make sure that you have a single direction to your references between layers. ie

Presentation -> Controllers -> Services -> Data

In your example you have one service reference another. BAD! you could move the logic up to a controller or add another layer

Presentation -> Controllers -> Application -> Services -> Data

Edit: Obviously, your question doesn't technically show a "circular reference" I am assuming you want a solution to the situation you show in your code examples, where you have unknown numbers of instances of the same service and potentially infinite loops or problems with instantiation where ServiceA calls ServiceB calls ServiceA again etc

10
  • 11
    Since when is it wrong to have references inside one layer?
    – Doc Brown
    Feb 19 at 19:10
  • 1
    Since someone needed any easy way to avoid "circular dependencies"
    – Ewan
    Feb 19 at 20:44
  • 5
    I'll be honest - this looks like a nigthmare to maintain
    – T. Sar
    Feb 20 at 11:33
  • This answer might be better if it addressed why that would be worth it? You could equally avoid circular dependencies by putting all your source into a single giant class but I don't think that would be worth it and isn't a good idea. Feb 20 at 19:45
  • 1
    @Mars the rule should guarantee a tree structure with no loops, which are obvs a danger if you have services calling services as in this case. i would put a logging service in the data layer myself. the application layer name comes from clean arch i believe, but its just an extra layer where you can put the "mashaling of services" logic and maintain the rule. Obvs you can have two things on the same layer call each other, but if you can avoid it then you have "an easy way to avoid circular dependencies"
    – Ewan
    Feb 21 at 10:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.