patterncppMinor
Is this a correct Factory Method implementation?
Viewed 0 times
thismethodfactorycorrectimplementation
Problem
I'm learning about design pattern and I tried to implement a Factory Method example, based on the GoF book.
Can I say that this is a correct implementation of it ? If not I will be glad to find out what are the pitfalls.
Can I say that this is a correct implementation of it ? If not I will be glad to find out what are the pitfalls.
#include
#include
using namespace std;
class Widget {
public:
virtual void draw() = 0;
};
class Win7Widget : public Widget {
public:
void draw() { cout win7fact(new Win7Factory);
unique_ptr win8fact(new Win8Factory);
vector widgets;
widgets.push_back(win7fact->Create());
widgets.push_back(win8fact->Create());
for(const auto& f : widgets)
{
f->draw();
}
return 0;
}Solution
Overall, it is pretty good.
The one thing you should improve is returning a smart pointer from the factory method. The way it is now, returning a raw pointer, it is up to the caller to decide how the memory management of the object is to be done. This is sub-optimal, as it leaves room for error. In your example, the widgets will leak, as nobody is explicitly owning that memory. So
Or use a type alias to make the code less verbose:
Avoid
The destructor of
No need to explicitly
The one thing you should improve is returning a smart pointer from the factory method. The way it is now, returning a raw pointer, it is up to the caller to decide how the memory management of the object is to be done. This is sub-optimal, as it leaves room for error. In your example, the widgets will leak, as nobody is explicitly owning that memory. So
Create() should return either a shared or unique pointer. Unique seems more fit for this case:virtual std::unique_ptr Create() = 0;Or use a type alias to make the code less verbose:
using WidgetPtr = std::unique_ptr;Avoid
using namespace std;. It is not that much more typing, and there are gains to it.The destructor of
Factory is empty, so you should make it a default (C++11 feature):virtual ~Factory() = default;No need to explicitly
return 0 from main(). The compiler adds it automatically for main if omitted.Code Snippets
virtual std::unique_ptr<Widget> Create() = 0;using WidgetPtr = std::unique_ptr<Widget>;virtual ~Factory() = default;Context
StackExchange Code Review Q#64740, answer score: 4
Revisions (0)
No revisions yet.