4

I recently found an example of using static member functions in pure abstract classes to initialize pointers to objects of its derived classes. I was wondering, if it's a design pattern or if it's a good programming practice? The code below is only an ilustration (e.g. both the defineRectangle() and defineCircle() member functions):

class Shape;

typedef std::unique_ptr<Shape> shape_ptr;

class Shape{

    public:

        Shape(){};
        virtual ~Shape(){};

        virtual void draw() const = 0;
        virtual float area() const = 0;

        virtual shape_ptr clone() const = 0;
        virtual shape_ptr create() const = 0;

        static shape_ptr defineRectangle(int, int );
        static shape_ptr defineCircle(float);
};

shape_ptr Shape::defineRectangle(int height, int width){
    shape_ptr ptrRectangle = shape_ptr(new Rectangle(height, width));
    return (ptrRectangle);
}

shape_ptr Shape::defineCircle(float radius){
    shape_ptr ptrCircle = shape_ptr(new Circle(radius));
    return (ptrCircle);
}

The final goal is to define a container of derived classes. For instance:

std::vector<std::unique_ptr<Shape> > vect;

and then we could add the derived classes in the container by either calling the static member functions of the Shape class:

vect.push_back(Shape::defineCircle(10));
vect.push_back(Shape::defineRectangle(5, 4));

or directly without any interface:

vect.push_back(std::unique_ptr<Shape>(new Circle(10)));
vect.push_back(std::unique_ptr<Shape>(new Rectangle(5,4)));

Which of both two ways of adding a derived class in a container should be preferred and why?
The full code can be found in the following link.
Any lights on it are really welcomed ;-)

Tin
  • 1,006
  • 1
  • 15
  • 27

3 Answers3

1

Since there is std::unique_ptr, I assume the compiler supports C++11. In that case, let me offer the third option:

vect.emplace_back(new Circle(10));
vect.emplace_back(new Rectangle(5,4));

(About .emplace_back: push_back vs emplace_back)

With this you don't need to repeat the shape_ptr, and you don't need to declare a new factory method to Shape whenever you add a new subclass.

Edit: In C++14, you can use std::make_unique to get rid of the raw new call.

vect.emplace_back(std::make_unique<Circle>(10));
vect.emplace_back(std::make_unique<Rectangle>(5, 4));
kennytm
  • 510,854
  • 105
  • 1,084
  • 1,005
  • This does not even answer the question. – Etienne de Martel Jan 22 '12 at 19:57
  • @EtiennedeMartel: How is this *not* answering the question "Which of both two ways of adding a derived class in a container should be preferred" – kennytm Jan 22 '12 at 20:06
  • @KennyTM : I'll give you a +1 ... I have no idea why I got downvoted either ... wish someone would explain ... – Jason Jan 22 '12 at 20:07
  • This leaks memory. – D. Dmitriy Aug 07 '18 at 13:01
  • @D.Dmitriy No it does not leak anything. Added a more modern alternative using `make_unique` anyway. – kennytm Aug 07 '18 at 14:04
  • @kennytm `emplace_back` may require memory reallocation, which may throw `std::bad_alloc`. In that case `std::unique_ptr` won't even be created, and Circle created with raw `new` won't ever be deallocated. Just to make sure I tested it with custom allocator which always throws and sure enough: Circle's destructor is never called. – D. Dmitriy Aug 07 '18 at 14:23
1

I was wondering, if it's a design pattern or if it's a good programming practice?

Yes, it's a variation on the factory pattern.

Basically put, it allows you to have a single method, that depending on the arguments to that method, will dispatch the dynamic creation of the correct derived object type. This allows you to use the same "factory" function in your code, and if there are any changes or additions to the underlying objects that the factory method creates, you do not have to change the code that is actually calling your "factory" function. Thus it's a form of encapsulation that isolates any changes for object creation to the segment of the code that is behind the "factory", not the code calling the "factory". For instance, using a factory, it's relatively trivial to add new types that the factory-method can create, but none of the previous code that is making a call to the factory has to change. You merely need to create a new derived class for the new object you want to create, and for any new code that desires that new object, you pass the correct new arguments. All the old arguments still work, and there are not changes that need to take place in the code with regards to returned pointer-types, etc.

The reason for using smart-pointers with the factory is to avoid memory leaks that can occur when pointer-ownership is ambiguous. For instance the factory has to return a pointer since it is dyanmically creating the object. The question then becomes who cleans up the pointer in order to avoid either dangling pointers or memory leaks? Smart pointers clear up this ownership problem, and guarantee that memory is not either inadvertently cleaned up when other objects are still pointing to that memory, or that the memory is not simply lost because the last pointer to that memory location goes out of scope without having delete called on it.

Jason
  • 31,834
  • 7
  • 59
  • 78
  • Care to comment on the reason for the downvote? I believe that I answered a specific question that the OP asked, and if I'm wrong, I would love to learn for future reference ... – Jason Jan 22 '12 at 20:00
  • I didn't voted, but I think the problem of this design is that it violates _single responsibility_ principle, as `Shape` both interface to Shape objects and Shapes factory. But you agreed, that it is good design :( – Lol4t0 Jan 22 '12 at 20:20
  • @Lol4t0 : I've seen in plenty of places though where the factory method is a static method of the abstract base-class. There are often some other additions like declaring all the derived class constructors `private`, and making the base-class a `friend` of all the derived classes, but I think these are C++ specific traits of the factory pattern vs. the general design of the pattern itself. For instance, Bruce Eckele in "Thinking in C++ Vol II" makes the factory-method a static method of the base-class. – Jason Jan 22 '12 at 20:27
  • What is the benefit not to separate interface itself and factory? I see no one for now, but may be I'm missing smth. – Lol4t0 Jan 22 '12 at 20:31
  • @Jason, so the idea is that we've a single method that selects which object to instantiate by using say a `switch-case` on a `std::string type`. here my doubt, where do we pass the parameters for the constructors of the derived classes? from what i understood the prototype for such a method looks like? `static std::unique_ptr factoryMethod(const std::string & type)`?, right? and where we pass the `radius` or `height/width` parameters? – Tin Jan 22 '12 at 20:36
  • @Tin : To get around the problem of different argument types, you could pass a functor or lambda that pre-binds your arguments, and then is used by the factor to create the correct object type with it's own set of custom arguments. So the functor would have a method to say what type it can create, and have pre-bound arguments to create the proper object. – Jason Jan 23 '12 at 04:20
  • @Jason, thanks. Do you mind posting a code snippet for illustrating your suggestion? – Tin Jan 23 '12 at 08:56
  • @Tin : [Here is some example code with comments](http://ideone.com/uKaf8) demonstrating a way to have a factory function that can take multiple arguments for each type, and call the appropriate construtors. I'm sure there is fancier template stuff that can be done, but this basically does the job, making any coding errors throw compiler errors rather than run-time crashes. Unfortunately there is a little bit of code reduplication because of the template class specializations, but it's not too bad for what you get. Let me know if you have any questions. – Jason Jan 23 '12 at 16:14
1

I'd advise against putting factory methods in the base class because, technically, Shape knows nothing about Rectangle or Circle. If you add a new shape, such as Donut, then what will you do? Add a new factory method to Shape? You'll clutter the interface in no time. So, IMO, the second method would be better.

If you want to reduce the verbosity of having to create shape_ptr a every time, you could always move the factory methods to the appropriate subclass:

class Circle : public Shape
{
    // ...
public:
    static shape_ptr make(float radius) 
    { 
        return shape_ptr(new Circle(radius)); 
    }
};

// ...

vect.push_back(Circle::make(5.0f));
Etienne de Martel
  • 34,692
  • 8
  • 91
  • 111
  • 1
    That defeats the whole point of the factory design pattern ... now you have to know the specific class that the static factory method is in, which means the same factory method can't dispatch any number of different derived objects. – Jason Jan 22 '12 at 20:05
  • What I'm saying is that I'm not sure a factory is even needed in this case. – Etienne de Martel Jan 22 '12 at 20:08
  • That's fine, but the OP is asking if the code examples he was referring to was a design-pattern (he then proceeded to provide an example) ... it is, and it's called a factory-pattern. What you're describing is not really a factory-pattern, and I think you need to be clearer about that. As I read the question right now, it sounds like you're answer is saying "Here's a better-way to-do the factory-pattern", and that's not really the case. – Jason Jan 22 '12 at 20:10
  • BTW, the whole point is to not add more methods, but to **dispatch** from a single method the correct pointer-type. So if you want to add more objects, you simply incorporate that addition into the single factory-method ... no increase in verbosity needed. – Jason Jan 22 '12 at 20:14
  • Yes, but I don't see why a factory method in this case would better than just instantiating a shape object directly. – Etienne de Martel Jan 22 '12 at 20:16