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?

• Euphoric: DO. NOT. USE. SINGLETONS!
• Doc Brown: @Euphoric: usage of singletons is as bad as writing sentences in capital letters ;-)
• Doc Brown: Badulake, can you explain what you mean by the term "event" here? Do you use it in a purely technical sense, as in "event driven system", or in a domain sense, like an occasion or a meeting? (I am trying to understand what the responsibility of the Event class might or should be).
• Neil: You're confusing Event/Audit/User database entities with other roles that the classes have. Make each class do exactly the CRUD operations for the database and nothing else to preserve single-responsibility status. If you want, those classes can also be responsible for associating its own instance with a child table (one-to-many relationship). Anything beyond this must be put in a separate class.
• Christophe: This one? stackoverflow.com/a/35134687/3723423

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.

• X.Otano: I edited the code. It is not static sorry
• Doc Brown: @Badulake: sorry, I don't see why you cannot make it static.
• X.Otano: I can not make it static. Event is an example of class, and it has a much more properties than x ,Also I have dozens of classes similar to Event that represents other items like (Users, GroupsOfUsers, Items, etc) so the problem is more general that my example of code reflects
• Doc Brown: @Badulake: in your example, getUserOfEvent does not access the x in Event, it accesses the x in User. So either give a better example, or read my answer again and tell us precisely why applying the SRP does not work for your case.

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
}