Design question - resolving circular dependency between objects

Question

I got myself into a circular dependency between two classes, and I'm trying to come up with a clean solution.

Here's the basic structure:

class ContainerManager {
    Dictionary<ContainerID, Container> m_containers;

    void CreateContainer() { ... }
    void DoStuff(ContainerID containerID) { m_containers[containerID].DoStuff(); }
}

class Container {
    private Dictionary<ItemID, Item> m_items;

    void SetContainerResourceLimit(int limit) { ... }

    void DoStuff() {
        itemID = GenerateNewID();
        item = new Item();
        m_items[itemID] = item;
        // Need to call ResourceManager.ReportNewItem(itemID);
    }
}

class ResourceManager {
    private List<ItemID> m_knownItems;

    void ReportNewItem(ItemID itemID) { ... }

    void PeriodicLogic() { /* need ResourceLimit from container of each item */ }
}

The ContainerManager is exposed as a WCF service: the external point through which clients can create Items and Containers. The ResourceManager needs to be aware of new Items that are created. It does background processing, and occasionally it requires information from the Item's Container.

Now, the Container needs to have the ResourceManager (to call ReportNewItem), which will be passed from the ContainerManager. The ResourceManager requires information from the Container, which it can only obtain using the ContainerManager. This creates a circular dependency.

I would prefer to initialize objects with interfaces (rather than concrete objects) so that I can later create mock objects for unit-tests (e.g. create a mock ResourceManager), but I'm still left with the problem that the CM requires an RM in its ctor, and the RM requires a CM in its ctor.

Obviously, this can't work, so I'm trying to come up with creative solutions. So far I have:

1) Pass to ReportNewItem the Container to be used, and have the ResourceManager use it directly. This is a pain because the ResourceManager persistently stores the ItemIDs it is aware of. This means that when initializing the ResourceManager after, say, a crash, I would have to re-provide it with all the Containers it needs.

2) Initialize either the CM or the RM in two phases: e.g.: RM = new RM(); CM = new CM(RM); RM.SetCM(CM); But this is ugly, I think.

3) Make ResourceManager a member of ContainerManager. Thus the CM can construct the RM with "this". This will work, but will be a pain during testing when I will want to create an RM mock.

4) Initialize CM with an IResourceManagerFactory. Have CM call Factory.Create(this), which will initialize the RM using "this", and then store the result. For testing, I can create a mock factory which will return a mock RM. I think this will be a good solution, but it's a bit awkward creating a factory just for this.

5) Break the ResourceManager logic into Container-specific logic, and have a distinct instance in each Container. Unfortunately, the logic really is cross-Container.

I think the "correct" way is to pull out some code into a third class that both CM and RM will depend on, but I can't come up with an elegant way of doing that. I came up with either encapsulating the "reported items" logic, or encapsulating the Component information logic, neither of which seems right.

Any insights or suggestions would be greatly appreciated.

Answer 1

How about solution 5, but having containers derive from a common base class that implements the cross-container logic you mentioned?

Answer 2

What you're looking for is an interface. An interface allows you to extract the structure/definition of a shared object to an outside reference, allowing it to be compiled independently from both the Container and ResourceManager classes, and dependent on neither.

When you create the Container, you'll have the ResourceManager you want the container to report to... pass it to the constructor, or set it as a property.

public interface IResourceManager {
    void ReportNewItem(ItemID itemID);
    void PeriodicLogic();
}


public class Container {
    private Dictionary<ItemID, Item> m_items;

    //  Reference to the resource manager, set by constructor, property, etc.
    IResourceManager resourceManager;

    public void SetResourceManager (IResourceManager ResourceManager) {
        resourceManager = ResourceManager;
    }

    public void DoStuff() {
        itemID = GenerateNewID();
        item = new Item();
        m_items[itemID] = item;
        resourceManager.ReportNewItem(itemID);
    }
}


public class ResourceManager : IResourceManager {
    private List<ItemID> m_knownItems;

    public void ReportNewItem(ItemID itemID) { ... }
    public void PeriodicLogic() { ... }
}


//  use it as such:
IResourceManager rm = ResourceManager.CreateResourceManager(); // or however
Container container = new Container();
container.SetResourceManager(rm);
container.DoStuff();

Expand this concept to each of your circular references.

* Update *

You don't need to remove all of the dependencies into an interface... it would be perfectly fine, for example, for a ResourceManager to know about/depend on a Container

Answer 3

With just your short snippet (a required constraint, I'm sure - but it's hard to know if ResourceManager could be turned into a singleton for example.) here's my quick thoughts

1) When ReportNewItem() is called, can you not just pass the container that the item is in to the ResourceManager? This way, the RM doesn't need to touch containermanager.

class Container {
    private IResourceManager m_rm; //.. set in constructor injection or property setter

    void DoStuff() {
        itemID = GenerateNewID();
        item = new Item();
        m_items[itemID] = item;
        m_rm.ReportNewItem(this, itemId);
    }
}

class ResourceManager {
    private List<ItemID> m_knownItems;
    private Dictionary<ItemID, Container> m_containerLookup;        

    void ReportNewItem(Container, ItemID itemID) { ... }

    void PeriodicLogic() { /* need ResourceLimit from container of each item */ }
}

2) I'm a fan of factories. In general, if construcing or retrieving the correct instance of a class is more than just new(), I like to put it in a factory for separation of concern reasons.

Answer 4

Thank you all for the answers.

jalexiou - I will look into KeyedCollection, thanks (sheesh, I really need to register so I can post comments).

James, as I wrote, I do want to work with interfaces (if nothing else, it simplifies unit-testing). My problem is that to initialize the actual ResourceManager I would need to pass the ComponentManager, and to initialize the CM I would need to pass the RM. What you suggested is basically a two-phase initialization which I referred to as solution 2. I would prefer to avoid such two-phase initialization, but perhaps I'm being too religious here.

Philip, I think that passing the Component to ReportNewItem would be exposing too much to the ResourceManager (since Component supports various operations that I would rather not be accesible to the ResourceManager).

However, thinking about it again, I can take the following approach:

class ComponentManager { ... }

class Component {
    private ComponentAccessorForResource m_accessor;
    private ResourceManager m_rm;

    Component(ResourceManager rm) {
        m_accessor = new ComponentAccessorForResource(this);
        m_rm = rm;
    }
    void DoStuff() {
        Item item = CreateItem();
        ResourceManager.ReportNewItem(item.ID, m_accessor);
    }
    int GetMaxResource() { ... }
 }

 class ComponentAccessorForResource {
     private Component m_component;
     ComponentAccessorForResource(Component c) { m_component = c; }
     int GetMaxResource() { return m_component.GetMaxResource(); }
 }

 ResourceManager rm = new ResourceManager();
 ComponentManager cm = new ComponentManager(rm);

This seems clean enough to me. Hope no one disagrees :)

My original objection to passing the Component (or indeed something like the accessor I proposed here) was that I would have to re-provide them to the ResourceManager upon initialization, since the ResourceManager persistently stores the Items it has. But as it turns out I have to re-initialize it with the Items anyway, so it's not a problem.

Thanks again for the good discussion!

Answer 5

James,

Yes, ComponentManager and ContainerManager are one and the same (the names in my real code are entirely different, and I was trying to choose "generic" names for the code snippets - and I got them confused). If there are any other details you think would help, let me know and I'll provide them. I was trying to keep the snippet concise.

You are correct that the ComponentManager is not directly involved in the Component/ResourceManager relationship. My problem is that I would like to be able to use a different ResourceManager for testing. One way of achieving this is to have the CM provide the RM to the Component (indeed, there is only one RM, so it has to be constructed by someone other than each Component).

The ComponentAccessor does little other than hiding the parts of Component that I don't want the ResourceManager to know about (while allowing the ResourceManager to be tested using a ComponentAccessorMock). The same thing could be achieved by having the Component implement an interface which exposes just the methods I want the RM to use. This is in fact what I did in my code, and I suspect it's what you referred to by "expose Component.GetMaxResource".

The code now looks roughly like so:

// Initialization:

RM = new RM();
CM = new CM(RM);   // saves RM as a member

//
// Implementation
//

// ComponentManager.CreateComponent
C = new Component(m_RM);  // saves RM as a member

// Component.CreateNewItem
{
    Item item = new Item();
    m_RM.ReportNewItem(this, item);
}

And ReportNewItem expects an interface that exposes the methods it needs. This seems fairly clean to me.

A possible alternative is to make the ResourceManager customizable using a Strategy pattern, but I'm not sure what that would buy me.

I'd be happy to hear what you (or anyone else, of course) think of this approach.