Reputation:
I have a container class to which i can send a Geometry object as constructor argument.
Geometry is a polymorphic class as interface for other geometric types like Sphere and Rectangle.
My question is that in Container class the constructor with arguments "Container(std::string str, Geometry* geometry)" can i code this in more flexible manner.
Whenever i will add a new SubClass to Geometry than i would need to write another IF condtion in Container class Constructor.
include "pch.h"
#include <iostream>
#include<fstream>
#include "Container.h"
#include "Geometry.h"
#include "Sphere.h"
#include "Rectangle.h"
#include "Container.h"
int main()
{
const char* fileName = "saved.txt";
Sphere sph;
Rectangle rect;
Container contSphere("ABC", &sph);
Container contRectangle("DEF", &rect);
Sphere* s = (Sphere*)contSphere.getGeomtry();
s->PrintGeom();
Rectangle* r = (Rectangle*)contRectangle.getGeomtry();
r->PrintGeom();
do
{
std::cout << '\n' << "Press a key to continue...";
} while (std::cin.get() != '\n');
}
///////////////////////////////////////////////////////////////////////////////////////////////
#pragma once
#include <string>
class Geometry
{
private:
std::string stdstringGeom;
std::string stdstrType;
public:
Geometry() : stdstringGeom("GeometyrString"), stdstrType("Geometry") {}
virtual ~Geometry() {}
virtual std::string getType()
{
return stdstrType;
}
virtual void PrintGeom()
{
std::cout << "geometry virtual function";
}
};
/////////////////////////////////////////////////////////////////////////////////
#pragma once
#include "Geometry.h"
class Sphere : public Geometry
{
private:
std::string stdstrSphere;
std::string stdstrType;
public:
Sphere() : Geometry() , stdstrSphere( "DefaultSphere") , stdstrType("Sphere") {}
Sphere( std::string str) : Geometry() , stdstrSphere(str) , stdstrType("Sphere"){}
void PrintGeom()
{
std::cout << "Sphere Virtual Function" << std::endl;
}
std::string getType()
{
return stdstrType;
}
};
///////////////// Defination for Constructor class////////////////////
#include "Geometry.h"
#include "Sphere.h"
#include "Rectangle.h"
class Container
{
private:
std::string stdstrCont;
Geometry* geom;
public:
Container() : stdstrCont("NoName") { geom = new Geometry; }
Container(std::string str, Geometry* geometry) : stdstrCont(str)
{
// I am doing this to avoid slicing and i want to do a deep copy.
if (geometry->getType() == "Sphere")
{
Sphere* sph = (Sphere*)geometry;
geom = new Sphere(*sph);
}
else if (geometry->getType() == "Rectangle")
{
Rectangle* rec = (Rectangle*)geometry;
geom = new Rectangle(*rec);
}
}
~Container()
{
if (geom != nullptr)
delete geom;
}
Geometry* getGeomtry()
{
return geom;
}
void PrintContainer()
{
std::cout << stdstrCont;
}
};
Upvotes: 0
Views: 140
Reputation: 36597
Your design is all backward, and a consequence is that you are making the Container
responsible for working out the type of all Geometry
objects passed to it, in order to copy them. That will make your Container
a maintenance nightmare - if someone creates another class derived from Geometry
, they can forget to modify Container
accordingly.
You've also omitted code that is relevant to your question (like virtual destructors) and included code that is irrelevant to the question (declaration of std::string
members, and initialisation of them in constructors, and other virtual functions).
Instead, it would be better to make the class Geometry
, and its derived classes, responsible for copying themselves.
At heart would be the Geometry
class itself
#include <memory> // for std::unique_ptr
class Geometry
{
public:
Geometry() {};
virtual ~Geometry() = default;
virtual std::unique_ptr<Geometry> Clone() const = 0;
};
(I've omitted the std::string
members for convenience). The derived classes then override the Clone()
function, viz;
class Sphere: public Geometry
{
public:
Sphere() : Geometry() {};
~Sphere() = default;
std::unique_ptr<Geometry> Clone() const {return std::unique_ptr<Geometry>(new Sphere(*this));};
};
// similarly for other shapes
The Clone()
function is pure virtual in Geometry
, so the derived classes cannot be instantiated unless you remember to override it.
The responsibility for cloning rests with Geometry
and its derived classes - and the compiler will helpfully diagnose an error if a class is derived from Geometry
that does not override the Clone()
function.
Then, all a Container
needs to do is have a member std::unique_ptr<Geometry>
or (if you intend to have a set of them) a std::vector<std::unique_ptr<Geometry> >
.
For the case where the Container
only needs a single Geometry
, the definition of Container
might be
class Container
{
public:
Container() : geom() {};
Container(Geometry *p) : geom(p->Clone()) {};
Container(const Container &c) : geom(c.geom->Clone()) {};
Container &operator=(const Container &c)
{
geom = c.geom->Clone(); // this will release the existing c.geom
return *this;
};
~Container() = default;
private:
std::unique_ptr<Geometry> geom;
};
The reason I've used std::unique_ptr<Geometry>
in the above (instead of Geometry *
as in your code) is that std::unique_ptr<>
avoids the need to explicitly decide when to destroy a Geometry
object.
The no-argument constructor of Container
initialises to containing a null Geometry
(i.e. a null pointer).
The constructor of Container
that accepts a Geometry *
clones the passed object, and does not assume ownership of it. This means the caller is responsible for the lifetime of the object it passes. This is consistent with your main()
, which constructs objects of automatic storage duration.
Note that I'm following the "rule of three" - if a non-default version of a copy constructor, copy assignment, or destructor is defined, then the other two should also be defined. The operator=()
in the above works in a manner consistent with the copy constructor (i.e. it clones objects in the Container
, rather than causing Geometry
objects to be shared between Container
s). The destructor is explicitly defined as the default, since the Container
is not explicitly managing lifetime of its members (the destructor of std::unique_ptr
does the work).
Upvotes: 2