1

I have the following circular dependency that I'd like to eliminate. I think I must have a flaw in my design. And I'd much appreciate any feedback on how to fix this.

My circular dependency comes from 3 important classes. Worker, WorkingContext, and TaskManager.

First, I have 2 Worker classes (Worker, and WorkerGroup). Each Worker is constructed with a WorkingContext object that stores state info.

class Worker 
{
    Worker(WorkingContext * context);
};

class WorkerGroup
{
};

Next, I have WorkingContext.

class WorkingContext
{
    // state info and various getters
    ...
    TaskManager * getTaskManager();
};

As shown, one of the things the WorkingContext stores is a TaskManager, which has a number of useful functions that depend on Worker. Here is a snippet

class TaskManager
{
    void assignCredit(Worker * worker, WorkerGroup * group, int credit);
    void revokeCredit(Worker * worker, int credit);
};

The circular dependency is that Worker.h needs to #include "WorkingContext.h", which needs to #include "TaskManager.h", which in turns needs to #include "Worker.h".

Right now it works since I'm using forward reference, but that really is a last resort.

Improve this question
1
  • 7
    Forward references aren't a last resort. For this kind of sensible logical coupling, they are the right thing to do. Commented Aug 16, 2016 at 19:16

2 Answers 2

Reset to default
1

If Task Manager does not hold state, you can pull it out of Working Context and make all its function static.

Otherwise, considering how little i know from the snippet i can only guess what will be a good design, but it seems to me a worker should not contain (indirectly) a task manager.

If the design is as such that every worker has a state-full context object specific to it, this object is not the right container for a task manager API, especially as it seems that the task manager uses a worker and not the other way.

Try expressing the logical dependencies using "Has a" and "Uses a" relationships to better determine an appropriate hierarchy. Note that "Uses a" does not necessarily imply containment or ownership.

Improve this answer
1
  • "task manager uses a worker, and not the other way". i'm not sure if that's right. maybe a better name for taskmanager would be creditManager. worker and taskmanager has a many to 1 relationship. each worker has 1 CreditManager. 1 CreditManager handles credit for many workers. Before a worker actually does anything, it needs to first check with his taskManager if he has enough credits to do it. if not, he just exits. if he does, then he works, and then calls revokeCredit. maybe a better name for that would be returnCredit in any case, hope this is more clear now Commented Aug 18, 2016 at 16:14
1

This design seems correct. In real life there are plenty of mutual dependencies like this.

As Sebastian pointed out in the comments, forward declaration of the classes are not a work around. They are simply needed in case of mutual dependencies. The only thing I'd change would be to use smart pointers instead of raw pointers.

You could however make the design more robust by using the mediator pattern to act as a kind of hub between the objects and to encapsulate the interactions between the different parts of the collaboration (see here the checklist and here an example). The consequence would be that Worker, Workgroup and TaskManager would not interact directly; each would only interact with the mediator. Intuitively, your WorkingContext could be a good start for the concrete mediator.

Improve this answer

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.