Remove circular dependency

Question

I have 2 classes that have dependencies.

public class AuditManager
{
      //set of methods
      public static AuditManager Singleton= new AuditManager();
      public  int AuditEvent(int x){
          Event event=new Event(x);
          event.SaveToDB();
       }
} 

Also I have a class:

public  class Event 
{
    int x;

    //set of methods

    public Event (int x)
    {
       this.x=x;
    }

    public User getUserOfEvent(int eventId){
        User usr = Respository.Get(eventId);
        Audit.Singleton.AuditEvent(usr.x); //circular dependency between Audit and event
        return usr;
    }

    public int SaveToDB()
   {
      new EventDal(){id=x}.Save(); //EventDal is used a lot in more Entity-type classes
   }

}

As you see, Audit uses classes like Event (responsibles of representing the model) And also , some actions in the model should be audited.

How is the best way to remove these circular dependencies?

Answer 1

The general problem here is the Event class having too many responsibilities. It should be a dumb information container, maybe with some persistence methods, but not more. Keeping any domain logic out of the class, the problem will vanish.

For example, this method:

    User getUserOfEvent(int eventId){
        User usr = Respository.Get(eventId);
        Audit.Singleton.AuditEvent(usr.x); 
        return usr;
    }

should not be part of the Event class, because it contains domain logic. In this example, it does not even use any member of the Event class, so there is no clear reason why it needs to be there. One can place it somewhere else, for example in a separate controller or helper class, which resolves the cyclic dependency.

Even if the method would use members of an Event, one can always pass the event object as a parameter to the method, which makes it possible to refactor the domain logic into another class. Maybe some class EventManager or EventController, or another class with better name, depending on the context. One just have to make sure the Event does not depend on the EventManager, only vice versa.

Here is an older answer of mine for a similar problem, showing three ways of using the SRP to remove a cyclic dependency in a similar, but more complex situatiion.

Answer 2

public interface IAuditManager {

  public int CreateEvent(int x);

}

public class MyAuditManager : IAuditManager
{
  //...your implementation of IAuditManager, doesn't matter if it's 
  //singleton
}

public class Event  //...or whatever class from your project
{
    public IAuditManager AuditManager { get; set; }
    int x;

    public Event(int x, IAuditManager auditManager) 
    {
        AuditManager = auditManager;
        this.x = x;
    }

    //...whatever you need to do here...
    //no circular dependencies anymore, because you're using
    //dependency injection here
}