resolving circular dependency when using a factory

Question

I have the following config to evaluate and am using a factory to get an object to a subclass of MathOperation based on type.

    class MathOperation {
    Operation GetOperationType();
    int Evaluate (config c);
    }

    config {
      type = min
      config {
        type =  average
        int x 
        int y
     }
      config {
        type = sum 
        int p
        int q
     }
    ...
    }

For instance if x = 10, y = 20, p = 10, q = 2 the answer is min(average(10, 20), sum(10, 2)) = 12.

I am running into a circular dependency issue because each subclass of MathOperation needs to include the factory to evaluate it's subconfig and the factory ofcoruse needs to include each subclass of MathOperation. How do I resolve this? This is what I currently have:

MathOperationFactory.h and cc

#include "average.h"
#include "min.h"
#include "sum.h"
std::unique_ptr<MathOperationObject> MakeObject(OperationType type) {
switch(type) {
case min : return MinOperation();
...
}
}

MinOperation.h and cc

#include "mathoperationfactory.h"    
int Evaluate(Config c) {
int minimum = 1000; // large number.
ASSERT(config.type = min);
for(config : c) // repeated configs {
  type t = c.type;
  factory.MakeObject(t);
  if(t.Evaluate < minimum) {
 minimum = t;
}
}
return minimum;
}

This seems like pseudocode rather than your real code. While it's good to cut down the real code rather than posting the entire program, you should feed the code you're posting into a compiler and make sure it exhibits the same problem you're asked about (and not a bunch of syntax errors). — Ben Voigt, May 22 '17 at 22:13
`I am running into a circular dependency issue because each subclass of MathOperation needs to include the factory to evaluate it's subconfig and the factory ofcoruse needs to include each subclass of MathOperation.` Search for information on forward-declarations. The respective headers should not need to include the full details of each other; that should only be done in the source files. — underscore_d, May 22 '17 at 22:44
Possible duplicate of [Resolve header include circular dependencies](https://stackoverflow.com/questions/625799/resolve-header-include-circular-dependencies) — drescherjm, May 22 '17 at 23:05

James Poag · Answer 1 · 2017-05-22T23:02:36.300

The Factory doesn't need to know the subtype, it just needs to be able to new one up. One way to do this is with a Creator class whose job is to delegate the creation of the concrete object back to the class itself.

I'm using std::string here for names, but you could easily use int or Operation enum.

Something like:

#pragma once

#include <string> // 
#include <map>
#include <typeinfo>

class MathOperation;

/************************************************************************/
/* MathOperation           Factory                                      */
/************************************************************************/
// Abstract Interface Type For Creator
struct CMathOperationCreator
{
    virtual MathOperation* Create() = 0;
    virtual ~CMathOperationCreator() {}
};

// Creator Map
std::map<std::string, CMathOperationCreator*, StringLessNoCaseCHAR>& GetMathOperationFactoryMap();

// Templated concrete creator, to be registered in the header of the concrete mathop type
template<class Derived>
struct CMathOperationConcreteCreator: public CMathOperationCreator
{
    CMathOperationConcreteCreator(const std::string& theMathOperationTypeId)
    {
        auto aFactoryItem = GetMathOperationFactoryMap().find(theMathOperationTypeId);
        if(aFactoryItem != GetMathOperationFactoryMap().end())
        {
            if(typeid(*aFactoryItem->second) == typeid(*this)) // avoid duplicates
                return;
        }
        GetMathOperationFactoryMap()[theMathOperationTypeId] = this;
    }
    virtual MathOperation* Create() {return new Derived();}
};

//Factory Method
MathOperation* CreateMathOperation(const std::string& theMathOperationTypeId);

/**
* Macro to automatically register a MathOperation Type
*/
#define REGISTER_MathOperation( ConcreteMathOperation, name ) \
    static CMathOperationConcreteCreator<ConcreteMathOperation> ConcreteMathOperation##Creator(name);

The CPP file:

// This is dumb, you don't have to do this, you just need a singleton factory that holds this map
std::map<std::string, CMathOperationCreator*, StringLessNoCaseCHAR>& GetMathOperationFactoryMap()
{
    static std::map<std::string, CMathOperationCreator*, StringLessNoCaseCHAR> theMap;
    return theMap;
}

MathOperation* CreateMathOperation( const std::string& theMathOperationTypeId )
{
    auto aFactoryItem = GetMathOperationFactoryMap().find(theMathOperationTypeId);
    if (aFactoryItem != GetMathOperationFactoryMap().end())
    {
        MathOperation* aObject = aFactoryItem->second->Create();
        return aObject;
    }

    return NULL;
}

Register a class:

class MinOperation : public MathOperation {
    Operation GetOperationType();
    int Evaluate (config c);
};
REGISTER_MathOperation(MinOperation, "min");

Then, when you're parsing your tokens, you can query the factory for the operation:

MathOperation* pOp = CreateMathOperation(token.lowercase());

As a caveat, you will need to add the header of each class to a cpp module somewhere so the linker doesn't strip your MathOperation types. — James Poag, May 22 '17 at 22:59

score 0 · Answer 2 · answered May 22 '17 at 22:36

As pointed out in the comments, it's hard to be sure without seeing real code. However, most likely the issue is you are putting too many includes in the header files. if you just add #include "mathoperationfactory.h" in the cc file, you should be fine.

Also, you need to use include guards.

score 0 · Answer 3 · answered May 22 '17 at 23:04

0

#pragma once makes sure that a header is only included once. Always put this as your first line in headers.

answered May 22 '17 at 23:04

James Poag

2,320
1
13
20

I don't see why this is a separate answer and not just part of your other one. Besides, `#pragma once` is not portable. Include guards implemented using `#ifndef` are. – underscore_d May 22 '17 at 23:14
This answer is similar to @dempzorz only instead of calling it `include guards` I went ahead and suggested `#pragma once` (as opposed to `#ifdef/#endif`). Also, this could conceivably fix his problem and is a stand alone separate answer to the question. – James Poag May 22 '17 at 23:17

resolving circular dependency when using a factory

3 Answers3