Nick
Nick

Reputation: 10559

Best method to implement an abstract factory pattern

Consider following code:

#include <stdio.h>


// =============================


class Shape{
public:
    virtual ~Shape(){};

    virtual void process() = 0;
};

class Triangle : public Shape{
public:
    virtual void process() override {
        printf("BBB\n");
    }
};


// =============================


/* option 1 */
class TriangleProducer{
public:
    Triangle factory(){
        return Triangle {};
    }
};



/* option 2 */
class PtrShapeProducer{
public:
    Shape *factory(){
        return new Triangle {};
    }
};



/* option 3 */
class PimplShape : public Shape{
    Shape *sh;
public:
    PimplShape(Shape *sh) : sh(sh){
    }

    virtual ~PimplShape() override{
        delete sh;
    }

    virtual void process() override {
        sh->process();
    }
};

class PimplShapeProducer{
public:

    PimplShape factory(){
        return new Triangle {};
    }
};


// =============================


int main(){
    TriangleProducer f1;
    Triangle tri = f1.factory();
    tri.process();



    PtrShapeProducer f2;
    Shape & sh = *f2.factory();
    sh.process();
    delete & sh;



    PtrShapeProducer f3;
    PimplShape psh = f3.factory();
    psh.process();



    return 0;
}

OPTION 1

It is nice, but it does not really achieve polymorphism. Return type is known and you must match it. One may add auto instead of Triangle, but this not change anything except easier refactoring.

OPTION 2

This is how Java and PHP is doing it. But I understood that "raw" pointers are not desirable in C++. One may add std::unique_ptr, but once again this does not change anything, except missing delete statement.

OPTION 3

This is what someone propose here some time ago - works nice, no "raw" pointers, no delete. But it is so much code, and way too complicated - seems fancy way, but not the correct one.

OPTION 4 (not implemented here)

Playing with const references - however they are const and it does not change the "factory" return type. I think, this is more like variation, it is not real option.

Any other option I am missing?
Also what would be best option to go?

Upvotes: 3

Views: 1436

Answers (2)

tp1
tp1

Reputation: 1207

Your factories are passing around ownership. There's another alternative to that aspect; instead of passing around ownership, you can make the factory own the objects:

   class Factory {
   public:
      ~Factory() { for(int i=0;i<vec.size();i++) delete vec[i]; }
      Shape &build_obj() {
         Shape *sh = new Triangle;
         vec.push_back(sh);
         return *sh;
       }

   private:
      void operator=(const Factory &);
      std::vector<Shape*> vec;
   };

Upvotes: 0

Chris Drew
Chris Drew

Reputation: 15334

I think the most idiomatic modern C++ method is the one you mention in passing but ignore. Return a std::unique_ptr<Shape>.

It is safe, clearly expresses ownership, supports polymorphism and doesn't need much code.

class ShapeFactory {
public:
  std::unique_ptr<Shape> create(){
    return std::make_unique<Triangle>();
  }
};

But I wouldn't want to claim it was the "best" method.

Your PimplShape in option 3 is actually very similar to a unique_ptr just less generic or tested.

Upvotes: 6

Related Questions