Reputation: 7699
In the below code, I write a factory method to create objects of a type in a class heirachy.
#include <iostream>
#include <memory>
using namespace std;
enum Type {
_Base, _A, _B, _C
};
class Base{
private:
Type type = _Base;
public:
virtual Type getType(){
return type;
}};
class A : public Base{
private:
Type type = _A;
public:
using Base::Base;
};
class B : public Base{
private:
Type type = _B;
public:
using Base::Base;
};
class C : public Base{
private:
Type type = _C;
public:
using Base::Base;
};
shared_ptr<Base> letterFactory(Type which){
shared_ptr<Base> base = make_unique<Base>(Base());
switch (which){
case _A:
base = make_unique<Base>(A());
case _B:
base = make_unique<Base>(A());
case _C:
base = make_unique<Base>(C());
}
return base;
}
int main(){
shared_ptr<Base> instanceOfA = letterFactory(_A);
cout << instanceOfA->getType() << endl;
shared_ptr<Base> instanceOfB = letterFactory(_B);
cout << instanceOfB->getType() << endl;
shared_ptr<Base> instanceOfC = letterFactory(_C);
cout << instanceOfC->getType() << endl;
return 0;
};
The output is
0
0
0
How can I make the output
1
2
3
Upvotes: 1
Views: 66
Reputation: 12293
I refactored your code a bit just to apply some of the best practices:
enum class
instead of the raw enum
because enum class
represents a scoped enumeration type and it is also strongly typed which means you cannot convert it that easily to integer as the raw enum
(that's why we have to_integral
template function)using namespace std;
because it imports the entirety of the std namespace into the current namespace of the programstd::make_shared
instead of std::make_unique
#include <memory>
#include <iostream>
enum class Type : int {
_Base = 0,
_A = 1,
_B = 2,
_C = 3
};
class Base{
private:
Type type = Type::_Base;
public:
virtual Type getType(){
return type;
}
};
class A : public Base{
private:
Type type = Type::_A;
public:
virtual Type getType() override {
return type;
}
};
class B : public Base{
private:
Type type = Type::_B;
public:
virtual Type getType() override {
return type;
}
};
class C : public Base{
private:
Type type = Type::_C;
public:
virtual Type getType() override {
return type;
}
};
std::shared_ptr<Base> letterFactory(Type which){
switch (which){
case Type::_A:
return std::make_shared<A>();
case Type::_B:
return std::make_shared<B>();
case Type::_C:
return std::make_shared<C>();
default:
return std::make_shared<Base>(Base());
}
}
template <typename Enum>
constexpr typename std::enable_if<std::is_enum<Enum>::value,
typename std::underlying_type<Enum>::type>::type
to_integral(Enum const& value) {
return static_cast<typename std::underlying_type<Enum>::type>(value);
}
int main(){
std::shared_ptr<Base> instanceOfA = letterFactory(Type::_A);
std::cout << to_integral(instanceOfA->getType()) << std::endl;
std::shared_ptr<Base> instanceOfB = letterFactory(Type::_B);
std::cout << to_integral(instanceOfB->getType()) << std::endl;
std::shared_ptr<Base> instanceOfC = letterFactory(Type::_C);
std::cout << to_integral(instanceOfC->getType()) << std::endl;
return 0;
};
Upvotes: 1
Reputation: 9855
Your factory is a bit flawed and your getType()
function is not overridden in the derived classes. I guess you wanted to do something along this lines:
#include <iostream>
#include <memory>
using namespace std;
enum Type {
_Base, _A, _B, _C
};
class Base{
public:
virtual ~Base() = default;
virtual Type getType() const {
return _Base;
};
};
class A : public Base{
public:
virtual Type getType() const override {
return _A;
};
};
class B : public Base{
public:
virtual Type getType() const override {
return _B;
};
};
class C : public Base{
public:
virtual Type getType() const override {
return _C;
};
};
unique_ptr<Base> letterFactory(Type which){
switch (which){
case _Base:
return make_unique<Base>();
case _A:
return make_unique<A>();
case _B:
return make_unique<B>();
case _C:
return make_unique<C>();
}
return nullptr;
}
int main(){
shared_ptr<Base> instanceOfA = letterFactory(_A);
cout << instanceOfA->getType() << endl;
shared_ptr<Base> instanceOfB = letterFactory(_B);
cout << instanceOfB->getType() << endl;
shared_ptr<Base> instanceOfC = letterFactory(_C);
cout << instanceOfC->getType() << endl;
return 0;
};
Notice that we got rid of the type
member completely and instead properly overrode the getType()
function. Furthermore, factory functions like this usually return unique_ptr
(which can be implicitly converted to shared_ptr
if you really want to).
Upvotes: 3
Reputation: 15951
Your Base
class has a member type
and a virtual member function getType()
that returns the value of the member type
. Your classes A
, B
, and C
derive from Base
. That means they all have a Base
subobject. That subobject contains the member Base::type
. In addition, they all also add another member type
that is then not ever used by anything. Also, none of them override the getType
method either. So whenever you call
instanceOfX->getType()
even if instanceOfX
points to an instance of one of the derived classes, since none of the derived classes overrite getType
, you'll end up calling Base::getType
, which will return the value of Base::type
, which is always _Base
…
What you actually wanted was probably something along the lines of:
struct Base
{
virtual Type getType() const = 0;
protected:
Base() = default;
Base(Base&&) = default;
Base(const Base&) = default;
Base& operator =(Base&&) = default;
Base& operator =(const Base&) = default;
~Base() = default;
};
class A : public Base
{
public:
Type getType() const override { return _A; }
};
class B : public Base
{
public:
Type getType() const override { return _B; }
};
class C : public Base
{
public:
Type getType() const override { return _C; }
};
Note that this is almost certainly bad design. The only purpose such a getType
method could serve is so that client code can find out the concrete type of the object that a Base*
it got is pointing to. If you ever need this information, your design violates the Liskov Substitution principle…
Apart from all that, note that _Base
, _A
, _B
, and _C
are reserved names [lex.name]/3 that you're not supposed to use in C++ code…
Upvotes: 2