0

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;
                        ^
StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
Aquarius_Girl
  • 21,790
  • 65
  • 230
  • 411
  • Change the return type to `PaintShapes *`. Also, consider renaming `PaintShapes` to `PaintShape`. – Sid S Jun 17 '18 at 03:29
  • Your function will need to either return a pointer (`PaintShapes *`) or an object which manages the pointer (various options depending on how the object is to be used and its liftetime managed, but one example is `std::unique_ptr`). Note that, although you have named your function as if it returns a set of objects, your implementation has it only returning one. – Peter Jun 17 '18 at 03:30
  • *My intention is to make an empty virtual function in the base class* -- Note that your base class virtual function still must return a value, since it is declared to return a value. – PaulMcKenzie Jun 17 '18 at 03:32
  • Change your base class into a pure virtual interface. Much better. – Martin G Jun 17 '18 at 06:08

1 Answers1

4

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.

Paul Sanders
  • 24,133
  • 4
  • 26
  • 48
  • I'd suggest the tiniest of improvements. Return a `std::unique_ptr` instead to handle the ownership properly. – StoryTeller - Unslander Monica Jun 17 '18 at 05:55
  • @StoryTeller Sure, but I wanted to keep things simple here for what I hope are obvious reasons. Also (and this is just a personal preference) I like to return a raw pointer from a factory method such as this as it is more flexible. The caller might want to manage it with a `shared_ptr`, for example. – Paul Sanders Jun 17 '18 at 06:01
  • 2
    For maximum flexibility you *should* return a `unique_ptr`. A standard shared pointer has a c'tor taking a unique pointer. A user working with other smart pointer types can just add a call to `release()` before assigning (assuming the library they use isn't already sane and interoperable with `std::`). And finally, no user is obligated to worry themselves about grabbing that pointer in an exception safe manner. It's already done for them. – StoryTeller - Unslander Monica Jun 17 '18 at 06:17
  • @StoryTeller Oh, OK. I consider myself educated, thank you. Added a note to the post. – Paul Sanders Jun 17 '18 at 06:34
  • So `PaintShape` has to know about every single derived class at compile time. I believe there are better ways to go about this, e.g. having derived classes register with the factory. – Sid S Jun 17 '18 at 06:38
  • @SidS I'm sure there are, _but please bear in mind the apparent level of experience of the OP_. – Paul Sanders Jun 17 '18 at 06:39
  • @Sid S - the GOF's factory method design pattern, in its simplest form, does involve some function (member of a factory class) that knows everything about the classes they construct at compile time. That is consistent with the approach here. There are other variants that allow more flexibility (e.g. allowing the types to be constructed to be determined at run time, rather than fixed at compile time). – Peter Jun 17 '18 at 06:55
  • @Peter This has got _absolutely nothing to do with the question_, but while idly Googling gang-of-four, I came across [this](https://dl.acm.org/citation.cfm?id=2550615), and in the abstract they have this to say "Moreover, authors of up-voted posts express significantly less negative emotions than authors of down-voted posts." Well, who'd have thought it? I'm only 25,000 behind you, work to do. And yes, I know, this should be on meta. – Paul Sanders Jun 17 '18 at 07:00
  • Does gof book also talk about the static function? Is this the best way to have factory method? – Aquarius_Girl Jun 17 '18 at 07:12
  • @PaulSanders - sometimes research is needed to conclude the seemingly obvious. However, I've seen some quite significant negativity from members here with many more points than either of us will ever have. I'd argue it is the people who give negative votes who are more likely to use negative language than the recipients - and those people will tend to have been previously awarded up-votes since, if they hadn't, they would be unable to vote. – Peter Jun 17 '18 at 07:28
  • @Aquarius_Girl - the static function is not usually mentioned by GOF, since they are focusing on design patterns, not any programming language. Static member functions are a feature of some programming languages (like C++) and not others. – Peter Jun 17 '18 at 07:30
  • @Aquarius_Girl I wouldn't know about the book itself (I've never actually read it and word is that it's getting as bit out of date now anyway) but in the code above the factory function _has_ to be static because you have no instance (aka object) yet to call it on. That's why I emphasised the fact, so I'm glad you picked up on it because it's an important detail. – Paul Sanders Jun 17 '18 at 07:32
  • @Peter _I've seen some quite significant negativity from members here with many more points than either of us will ever have_. Yes! I hate the way the inexperienced can get put down - bullied, even - on this site sometimes. We were all young once. – Paul Sanders Jun 17 '18 at 07:34
  • Is there any other way of implementing factory method without making it static? Is this the only way? – Aquarius_Girl Jun 17 '18 at 08:02
  • @Aquarius_Girl Is making it `static` a problem? I'm not sure why you're concerned about this, perhaps you could share that with us. – Paul Sanders Jun 17 '18 at 08:24
  • No not a problem. I just want to know what are the possible ways. So That we can compare which method is better and why. – Aquarius_Girl Jun 17 '18 at 08:58
  • OK, well, maybe [this](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) will help. – Paul Sanders Jun 17 '18 at 09:11