I am going over a number of design patterns and today I ask you to please inspect my application attempt of the Factory design pattern. Please feel free to share what you think about it, because my goal is to improve my skills. I am using g++ 4.8.5. Note: this is not homework, just practice.
In short, several classes are derived from CShapeBase and these are produced by CFactory.cpp and returned to main.cpp; main.cpp is not aware of which class is instantiated, which appears to be the purpose of the factory design pattern.
Compiling:
g++ -Wall -fexceptions -g -std=c++11 -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/03_Factory_Pattern/CCircle.cpp -o obj/Debug/CCircle.o
g++ -Wall -fexceptions -g -std=c++11 -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/03_Factory_Pattern/CFactory.cpp -o obj/Debug/CFactory.o
g++ -Wall -fexceptions -g -std=c++11 -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/03_Factory_Pattern/CHexagon.cpp -o obj/Debug/CHexagon.o
g++ -Wall -fexceptions -g -std=c++11 -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/03_Factory_Pattern/CPentagon.cpp -o obj/Debug/CPentagon.o
g++ -Wall -fexceptions -g -std=c++11 -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/03_Factory_Pattern/CRectangle.cpp -o obj/Debug/CRectangle.o
g++ -Wall -fexceptions -g -std=c++11 -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/03_Factory_Pattern/CSquare.cpp -o obj/Debug/CSquare.o
g++ -Wall -fexceptions -g -std=c++11 -c /mnt/home/Data_MaOt/Short_C_and_Cpp_progs/DesignPatterns/03_Factory_Pattern/main.cpp -o obj/Debug/main.o
g++ -o bin/Debug/03_Factory_Pattern obj/Debug/CCircle.o obj/Debug/CFactory.o obj/Debug/CHexagon.o obj/Debug/CPentagon.o obj/Debug/CRectangle.o obj/Debug/CSquare.o obj/Debug/main.o
CCircle.cpp
#include <iostream>
#include "CCircle.h"
CCircle::CCircle()
{
std::cout << "CCircle::CCircle(): Started and finished." << std::endl;
}
void CCircle::Identify()
{
std::cout << "CCircle::CCircle(): I am a CCircle object." << std::endl;
}
CCircle.h
#ifndef CCIRCLE_H
#define CCIRCLE_H
#include "CShapeBase.h"
class CCircle : public CShapeBase
{
public:
CCircle();
void Identify();
};
#endif // CCIRCLE_H
CFactory.cpp
#include <iostream>
#include <memory>
#include "CFactory.h"
#include "CCircle.h"
#include "CSquare.h"
#include "CRectangle.h"
#include "CPentagon.h"
#include "CHexagon.h"
std::shared_ptr<CShapeBase> CFactory::Produce()
{
std::shared_ptr<CShapeBase> myBasePtr = nullptr;
while(myBasePtr == nullptr)
{
std::cout << "Please select your shape:" << std::endl;
std::cout << "1) Circle" << std::endl;
std::cout << "2) Square" << std::endl;
std::cout << "3) Rectangle" << std::endl;
std::cout << "4) Pentagon" << std::endl;
std::cout << "5) Hexagon" << std::endl;
int input = 0;
std::cin >> input;
switch(input)
{
case 1:
myBasePtr = std::make_shared<CCircle>();
break;
case 2:
myBasePtr = std::make_shared<CSquare>();
break;
case 3:
myBasePtr = std::make_shared<CRectangle>();
break;
case 4:
myBasePtr = std::make_shared<CPentagon>();
break;
case 5:
myBasePtr = std::make_shared<CHexagon>();
break;
default:
std::cout << "CFactory::Produce(): Did not produce a shape." << std::endl;
break;
}
}
return myBasePtr;
}
CFactory.h
#ifndef CFACTORY_H
#define CFACTORY_H
#include <memory>
#include "CShapeBase.h"
// Include derives classes here?
class CFactory
{
public:
std::shared_ptr<CShapeBase> Produce();
};
#endif // CFACTORY_H
CHexagon.cpp
#include <iostream>
#include "CHexagon.h"
CHexagon::CHexagon()
{
std::cout << "CHexagon::CHexagon(): Started and finished." << std::endl;
}
void CHexagon::Identify()
{
std::cout << "CHexagon::CHexagon(): I am a CHexagon object." << std::endl;
}
CHexagon.h
#ifndef CHEXAGON_H
#define CHEXAGON_H
#include "CShapeBase.h"
class CHexagon : public CShapeBase
{
public:
CHexagon();
void Identify();
};
#endif // CHEXAGON_H
CPentagon.cpp
#include <iostream>
#include "CPentagon.h"
CPentagon::CPentagon()
{
std::cout << "CPentagon::CPentagon(): Started and finished." << std::endl;
}
void CPentagon::Identify()
{
std::cout << "CPentagon::CPentagon(): I am a CPentagon object." << std::endl;
}
CPentagon.h
#ifndef CPENTAGON_H
#define CPENTAGON_H
#include "CShapeBase.h"
class CPentagon : public CShapeBase
{
public:
CPentagon();
void Identify();
};
#endif // CPENTAGON_H
CRectangle.cpp
#include <iostream>
#include "CRectangle.h"
CRectangle::CRectangle()
{
std::cout << "CRectangle::CRectangle(): Started and finished." << std::endl;
}
void CRectangle::Identify()
{
std::cout << "CRectangle::CRectangle(): I am a CRectangle object." << std::endl;
}
CRectangle.h
#ifndef CRECTANGLE_H
#define CRECTANGLE_H
#include "CShapeBase.h"
class CRectangle : public CShapeBase
{
public:
CRectangle();
void Identify();
};
#endif // CRECTANGLE_H
CShapeBase.h
#ifndef CSHAPEBASE_H
#define CSHAPEBASE_H
class CShapeBase
{
public:
virtual void Identify() = 0;
};
#endif // CSHAPEBASE_H
CSquare.cpp
#include <iostream>
#include "CSquare.h"
CSquare::CSquare()
{
std::cout << "CSquare::CSquare(): Started and finished." << std::endl;
}
void CSquare::Identify()
{
std::cout << "CSquare::CSquare(): I am a CSquare object." << std::endl;
}
CSquare.h
#ifndef CSHAPE_H
#define CSHAPE_H
#include "CShapeBase.h"
class CSquare : public CShapeBase
{
public:
CSquare();
void Identify();
};
#endif // CSHAPE_H
main.cpp
#include <iostream>
#include "CFactory.h"
int main()
{
std::cout << "main(): Started." << std::endl;
CFactory myFactory;
std::shared_ptr<CShapeBase> myBasePtr = myFactory.Produce();
myBasePtr->Identify();
std::cout << "main(): Finished." << std::endl;
return 0;
}
-
\$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers . \$\endgroup\$Peilonrayz– Peilonrayz ♦2017年04月03日 14:15:48 +00:00Commented Apr 3, 2017 at 14:15
1 Answer 1
Sure. You implemented the basic factory pattern.
But factories don't get useful to you start using Abstract Factories
. SO you can swap out out factory for another factory.
Side Note: When presenting code it's easier to review of the header comes before the source. And best if the base class comes first. Prevents a lot of scrolling backwards and forwards.
Virtual Functions
class CShapeBase
{
public:
virtual void Identify() = 0;
};
If a class has a virtual function and it will be dynamically created and destroyed via a pointer.
CShapeBase* object = new Circle();
... // Note: Demo use smart pointers in reality.
delete object;
Then you should also have a virtual destructor. As it currently stands your code will call the destructor of CShapeBase
but because the destructor is not virtual it will not call the destructor of Circle
.
Overiding virtual functions
When you override a virtual function its a good idea to mark it as overridden. That way if things in the future change your compiler will catch any changes and prevent compilation until you fix the update.
class CSquare : public CShapeBase
{
public:
CSquare();
void Identify() override;
// ^^^^^^^^
};
Ownership
std::shared_ptr<CShapeBase> CFactory::Produce()
Are you sure you want a shared_ptr
? The factory is not retaining ownership so there is no dual ownership.
Personally I would return unique_ptr
. The user of the code can then decide if they want to upgrade from unique_ptr
to shared_ptr
if that's what they require (as you can assign a unique_ptr
to a shared_ptr
.
The reason I would use unique_ptr
is that there is basically no overhead from unique_ptr
while shared_ptr
has a significant overhead because of the need to track weak_ptr
internally.