Search code examples
c++oopfactorygang-of-four

Is this the best way to write my factory method


My intention is to make an empty virtual function in the base class. Redefine that function in the derived classes such that they return the particular subclass's object.

createShapeObjects is the factory method here.

What is the proper implementation for factory method according to GOF book?

facto.h

#ifndef FACTO
#define FACTO

class PaintShapes
{
    public:
        virtual PaintShapes createShapeObjects(string arg) {};
};

class PaintTriangle : public PaintShapes
{
public:
    PaintTriangle() {}

    virtual PaintShapes createShapeObjects(string arg)
    {
        std::cout << "ddd";
        if (arg == "triangle")
            return new PaintTriangle;
    }
};

class PaintRectangle : public PaintShapes
{
public:
    PaintRectangle() {}

    virtual PaintShapes createShapeObjects(string arg)
    {
        std::cout << "eee";
        if (arg == "rectangle")
            return new PaintRectangle;
    }
};


/////
// My class which wants to paint a triangle:
/////

class MyClass
{
public:
    PaintShapes obj;
    void MyPaint()
    {
        obj.createShapeObjects("triangle");
    }
};



#endif // FACTO

main.cpp

#include <iostream>

using namespace std;
#include "facto.h"
int main()
{
    cout << "Hello World!" << endl;

    MyClass obj;
    obj.MyPaint();
    return 0;
}

This gives the error:

error: could not convert '(operator new(4u), (<statement>, ((PaintTriangle*)<anonymous>)))' from 'PaintTriangle*' to 'PaintShapes'
             return new PaintTriangle;
                        ^

Solution

  • I don't understand any of this.

    The purpose of a factory method is to create an instance of a derived class without calling its constructor directly. But your code is in a Catch-22 situation - to make, for example, a PaintRectangle you need to have an existing instance of such an object in the first place! I hope you can see this is going nowhere.

    Try something like this instead:

    class PaintShape
    {
    public:
        static PaintShape *createShapeObject(std::string shape);
    };
    
    class PaintTriangle : public PaintShape
    {
    public:
        PaintTriangle() { }
        // ...
    };
    
    class PaintRectangle : public PaintShape
    {
    public:
        PaintRectangle() { }
        // ...
    };
    
    //  This is our (*static*) factory method 
    PaintShape *PaintShape::createShapeObject(std::string shape)
    {
        if (shape == "triangle")
            return new PaintTriangle;
        if (shape == "rectangle")
            return new PaintRectangle;
        return nullptr;
    };
    

    And then you can simply do (for example):

    std::string shape;
    std::cout << "What shape would you like? ";
    std::getline (std::cin, shape);
    PaintShape *ps = PaintShape::createShapeObject (shape);
    // ...
    

    Please let me know if you have any questions - and please read the comments about why, strictly speaking, createShapeObject() should return a std::unique_ptr.