CODEWITHSUNDEEP
c++factory-pattern
Teodora
Teodora

Reputation: 727

factory pattern - invalid use of member in static member function

I'm new to this factory pattern in C++ and I was given the header yet while trying to implement one of the methods I am getting the following error:

Invalid use of member 'creationFunctions' in static member function

typedef Shape (*createShapeFunction)(void);
/* thrown when a shape cannot be read from a stream */
class WrongFormatException { };

class ShapeFactory {

public:

    static void registerFunction(const std::string &string, const createShapeFunction *shapeFunction);
    static Shape *createShape(const std::string &string);
    static Shape *createShape(std::istream &ins);

private:

    std::map<std::string, createShapeFunction *> creationFunctions;
    ShapeFactory();
    static ShapeFactory *getShapeFactory();
};


void ShapeFactory::registerFunction(const std::string &string, const createShapeFunction *shapeFunction)
{
    creationFunctions.at(string) = shapeFunction;
}

Upvotes: 0

Views: 7269

Answers (2)

Danny
Danny

Reputation: 2281

One problem I see is that you are accessing a variable private to ShapeFactory from a static function. Static functions are not tied to a specific instance of a ShapeFactory so you cannot access non-static private variables. Another side effect of this is that this is not defined in the context of static functions. Each time you create an instance of ShapeFactory a new instance of std::map<std::string, createShapeFunction *> creationFunctions is created for each ShapeFactory.

I'm not very familier with implementing factories, but I think what you might want to do is make std::map<std::string, createShapeFunction *> creationFunctions static so that it can be accessed by the static member functions of ShapeFactory. A private constructor like what your header file contains suggests that ShapeFactory will be a singleton and so if you really want it to be a singleton, making std::map<std::string, createShapeFunction *> creationFunctions by static should be okay.

Edit based on requirement to not modify header:

You can have registerFunction get a pointer to a factory from getShapeFactory(). Notice how I create a new pointer that is not a const pointer. This allows you to insert the pointer into the map. I'm not sure how best to handle shapeFunction as this method seems like it may not be ideal, but I'm not 100% sure what this function is supposed to do.

void ShapeFactory::registerFunction(const std::string &string, const createShapeFunction *shapeFunction)
{
    createShapeFunction* shapeFuncPtr = new createShapeFunction(*shapeFunction);
    ShapeFactory* factory = getShapeFactory();
    factory->creationFunctions.insert(std::pair<string, createShapeFunction*>(string, shapeFuncPtr));
}

Have a static variable within getShapeFactory that creates a factory. The value of this variable will get saved each time this function is called so after the first call of this function the same pointer will be returned each time. This has to be implemented in a .cpp file that contains your header.

ShapeFactory::getShapeFactory()
{
    static ShapeFactory* factory;
    if (factory == NULL)
        factory = new ShapeFactory();
    return factory;
}

Upvotes: 1

Paul Michalik
Paul Michalik

Reputation: 4381

You can't access non-static members of a class from a static member function. In your example the probably best idea would be to make all member functions non-static, except "getShapeFactory" which would then act as Mayer's singleton generator...

Upvotes: 2

Related Questions