Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C++ > How can I remove dynamic_cast and if statements from this code snippet?

Reply
Thread Tools

How can I remove dynamic_cast and if statements from this code snippet?

 
 
Chris Stankevitz
Guest
Posts: n/a
 
      11-16-2011
On Nov 16, 1:56*pm, Ian Collins <(E-Mail Removed)> wrote:
> You can always replace conditionals with virtual member functions.


I assume in the drawing case you are suggesting I add a virtual
function "GetVerticies" or a virtual function "Draw". This is now
what I want for these reasons:

1. I have to modify the Shape class to be aware that it is being drawn
2. Some shapes cannot be drawn with "verticies" and need to be drawn
carefully and uniquely -- for example a doughnut that has a hole in
the middle.

Thank you,

Chris
 
Reply With Quote
 
 
 
 
Ian Collins
Guest
Posts: n/a
 
      11-16-2011
On 11/17/11 11:03 AM, Chris Stankevitz wrote:
> On Nov 16, 1:56 pm, Ian Collins<(E-Mail Removed)> wrote:
>> You can always replace conditionals with virtual member functions.

>
> I assume in the drawing case you are suggesting I add a virtual
> function "GetVerticies" or a virtual function "Draw". This is now
> what I want for these reasons:
>
> 1. I have to modify the Shape class to be aware that it is being drawn
> 2. Some shapes cannot be drawn with "verticies" and need to be drawn
> carefully and uniquely -- for example a doughnut that has a hole in
> the middle.


So how would a generic drawing class be expected to know this? It would
have to changed for each new shape.

Does your window manager know how to render every type of window on your
system?

--
Ian Collins
 
Reply With Quote
 
 
 
 
Chris Stankevitz
Guest
Posts: n/a
 
      11-16-2011
On Nov 16, 2:12 pm, Ian Collins <(E-Mail Removed)> wrote:
> So how would a generic drawing class be expected to know this?


This is basically the question I am asking this group.

Below is a pseudocode answer to the question that
- leaves Shape, Circle, and Square untouched (good)
- places no drawing dependency on the shapes (good)
- leaves drawing code entirely in the Drawer classes (good)
- does not have switch or if statements (good)
* does use a dynamic cast (BAD)

Can you (or anyone else) come up with some c++ pseudocode that
- leaves Shape, Circle, and Square untouched (good)
- places no drawing dependency on the shapes (good)
- leaves drawing code entirely in the Drawer classes (good)
- does not have switch or if statements (good)
- does not use a dynamic cast (good)

Chris

=====

//----------
struct Drawer
{
virtual void Draw(Shape* shape) = 0;
};

//----------
struct CircleDrawer : public Drawer
{
void Draw(Shape* shape)
{
// dynamic_cast<Circle*>(shape)
// draw the circle
}
};

//----------
struct SquareDrawer : public Drawer
{
void Draw(Shape* shape)
{
// dynamic_cast<Square*>(shape)
// draw the square
}
};

//----------
struct DrawerCreator
{
virtual Drawer* GetNewDrawer() = 0;
virtual bool CanDraw(Shape* shape) = 0;
};

//----------
struct CircleDrawerCreator : public DrawerCreator
{
Drawer* GetNewDrawer() { return new CircleDrawer; }
bool CanDraw(Shape* shape) { return dynamic_cast<Circle*>(shape); }
};

//----------
struct SqareDrawerCreator : public DrawerCreator
{
Drawer* GetNewDrawer() { return new SquareDrawer; }
bool CanDraw(Shape* shape) { return dynamic_cast<Square*>(shape); }
};

//----------
struct DrawerManager
{
void RegisterDrawerCreator(DrawerCreator* dc)
{
drawers.push_back(dc);
}

void Draw(Shape* shape)
{
if (Drawer* drawer = GetDrawer(shape))
{
drawer->Draw(shape);
}
}

Drawer* GetDrawer(Shape* shape)
{
for (int i = 0; i < drawers.size(); ++i)
{
if (drawers[i]->CanDraw(shape))
{
return drawers[i]->GetNewDrawer();
}
}

return 0;
}

vector<Drawer*> drawers;
};

//----------
int main()
{
DrawerManager drawer_manager;

drawer_manager.RegisterDrawerCreator(new CircleDrawerCreator);
drawer_manager.RegisterDrawerCreator(new SquareDrawerCreator);

Shape* shape1 = new Circle;
Shape* shape2 = new Square;

drawer_manager.Draw(shape1);
drawer_manager.Draw(shape2);
}
 
Reply With Quote
 
Werner
Guest
Posts: n/a
 
      11-17-2011
On Nov 17, 1:20*am, Chris Stankevitz <(E-Mail Removed)>
wrote:

> Below is a pseudocode answer to the question that
> *- leaves Shape, Circle, and Square untouched (good)
> *- places no drawing dependency on the shapes (good)
> *- leaves drawing code entirely in the Drawer classes (good)
> *- does not have switch or if statements (good)
> ** does use a dynamic cast (BAD)
>
> Can you (or anyone else) come up with some c++ pseudocode that
> *- does not use a dynamic cast (good)


I can't see why using dynamic cast is so bad. It is not as
if you have an if/else style of programming that requires
constant modification. dynamic_cast under your control that
requires no modification is good (except for being a tid
slower, perhaps).

You might want to remove the burden from your client though (see
below):

#include <vector>

struct Shape{};
struct Circle : Shape{};
struct Square : Shape{};

struct AbstractDrawer
{
virtual void Draw(Shape* shape) = 0;

};

template <class ShapeT>
struct TypedDrawer : AbstractDrawer
{
typedef ShapeT shape_type;

protected:
virtual void DoDraw( shape_type& shape ) = 0;
//{ Default implementation ??? }

private:
virtual void Draw( Shape* shape )
{
DoDraw( dynamic_cast<shape_type&>(*shape) );
}
};

struct CircleDrawer : TypedDrawer<Circle>
{
protected:
virtual void DoDraw( Circle& shape )
{
//Implementation
}
};

struct SquareDrawer : TypedDrawer<Square>
{
protected:
virtual void DoDraw( Square& shape )
{
//Implementation
}
};

//----------
struct AbstractDrawerCreator
{
virtual AbstractDrawer* GetNewDrawer() = 0;
virtual bool CanDraw(Shape* shape) = 0;
};

template <class DrawerT>
struct TypedDrawerCreator : AbstractDrawerCreator
{
private:
typedef DrawerT drawer_type;
typedef typename DrawerT::shape_type shape_type;

virtual drawer_type* GetNewDrawer() const
{
return new drawer_type;
}
virtual bool CanDraw( Shape* shape ) const
{
return dynamic_cast<shape_type*>(shape);
}
};

typedef TypedDrawerCreator<CircleDrawer> CircleDrawerCreator;
typedef TypedDrawerCreator<SquareDrawer> SquareDrawerCreator;
//etc...


//[Werner Erasmus: Left this in as I've removed compiler
// errors]
struct DrawerManager
{
void RegisterDrawerCreator(AbstractDrawerCreator* dc)
{
drawers.push_back(dc);
}

void Draw(Shape* shape)
{
if (AbstractDrawer* drawer = GetDrawer(shape))
{
drawer->Draw(shape);
}
}

AbstractDrawer* GetDrawer(Shape* shape)
{
for (int i = 0; i < drawers.size(); ++i)
{
if (drawers[i]->CanDraw(shape))
{
return drawers[i]->GetNewDrawer();
}
}

return 0;
}

std::vector<AbstractDrawerCreator*> drawers;
};



 
Reply With Quote
 
Francesco
Guest
Posts: n/a
 
      11-17-2011
On 16 Nov, 19:37, Chris Stankevitz <(E-Mail Removed)> wrote:
> Hello,
>
> I would like to remove the "dynamic_cast" and "if" statements from the
> code below. *I believe some people would describe this as making the
> code "polymorphic". *Can you recommend a way to do this without
> modifying the original classes in "Library A"?
>
> My intention is to create an "XML Writer" class(es) for shapes without
> putting "XML code" in the base classes. *I plan to use a similar
> pattern for drawing and other tasks my application has to perform on
> Shape objects.
>
> Thank you,
>
> Chris
>
> // Library A
> struct Shape { virtual ~Shape() {} };
> struct Circle : public Shape { float radius; };
> struct Square : public Shape { float edge; };
>
> // Library B
> #include <iostream>
>
> class XmlWriter
> {
> * static void write(Shape* shape)
> * {
> * * if (Circle* circle = dynamic_cast<Circle*>(shape))
> * * {
> * * * std::cout << "<Circle Radius=" << circle->radius << "/>";
> * * }
> * * else if (Square* square = dynamic_cast<Square*>(shape))
> * * {
> * * * std::cout << "<Square Edge=" << square->edge << "/>";
> * * }
> * }
>
>
>
>
>
>
>
> };


Hi,

IMHO you can try one of the following approaches:
- map the type_info of the classes to some function that does the
actual work, any map will give you on the average better than linear
behavior (cascade of ifs). This applies only if you have lots of
classes, if you have only a handful it will be worse (probably).
- create a mirror hierarchy of classes (deriving or composing). Add
the virtual write method on those classes and just use those instead
of the original ones...

Obviously if you could modify the original classes, that would be the
way to go...
Bye,
Francesco

// CODE


//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~
// Library A
struct Shape { virtual ~Shape() {} };
struct Circle : public Shape { float radius; };
struct Square : public Shape { float edge; };

//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~
// Library B
#include <iostream>
#include <cassert>

#define USE_UNORDERED_MAP

#ifdef USE_UNORDERED_MAP
#include <tr1/unordered_map>
#else
#include <map>
#endif

//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~
// Type unsafe functions!!

void WriteCircle( Shape * shape )
{
Circle * circle = static_cast< Circle * >( shape );
std::cout << "<Circle Radius=" << circle->radius << "/>";
}

void WriteSquare( Shape * shape )
{
Square * square = static_cast< Square * >( shape );
std::cout << "<Square Edge=" << square->edge << "/>";
}

//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~
// TypeInfo class for map key

class CTypeInfo
{
public:
CTypeInfo( std::type_info const & inTypeInfo )
: mTypeInfoPtr( &inTypeInfo )
#ifdef USE_UNORDERED_MAP
, mHash( std::tr1::hash< std::string >()
( inTypeInfo.name() ) )
#endif
{ }

bool operator<( CTypeInfo const & inTypeInfo ) const
{ return mTypeInfoPtr->before( *inTypeInfo.mTypeInfoPtr ); }

bool operator==( CTypeInfo const & inTypeInfo ) const
{ return *mTypeInfoPtr == *inTypeInfo.mTypeInfoPtr; }

#ifdef USE_UNORDERED_MAP
size_t GetHash( void ) const
{ return mHash; }
#endif

private:
std::type_info const * mTypeInfoPtr;
#ifdef USE_UNORDERED_MAP
size_t mHash;
#endif

};

//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~
// Map stuff

#ifdef USE_UNORDERED_MAP
struct CTypeInfoHash
{
size_t operator()( CTypeInfo const & inTypeInfo ) const
{ return inTypeInfo.GetHash(); }
};
#endif

typedef void (*tFuncPtr)( Shape * );
#ifdef USE_UNORDERED_MAP
typedef std::tr1::unordered_map< CTypeInfo, tFuncPtr, CTypeInfoHash >
CMap;
#else
typedef std::map< CTypeInfo, tFuncPtr > CMap;
#endif


//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~
// XmlWriter

class XmlWriter
{
private:
// this is all thread unsafe!!!
static CMap & GetMap()
{
static CMap sMap;
return sMap;
}

static void Init()
{
static bool sInit = false;
if( !sInit )
{
CMap & map = GetMap();
map.insert( CMap::value_type( typeid( Circle ),
&WriteCircle ) );
map.insert( CMap::value_type( typeid( Square ),
&WriteSquare ) );
sInit = true;
}
}
public:
void write(Shape* shape)
{
Init();
CMap & map = GetMap();
CMap::const_iterator iter = map.find( typeid( *shape ) );
assert( iter != map.end() );
iter->second( shape );
}
};

//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~
// augmenting classes
// WARNING: quick example, many things to look after...

struct CShapePlus
{
virtual ~CShapePlus() {}
virtual void Write() const = 0;
};

// composition

struct CCirclePlus : public CShapePlus
{
Circle mCircle;
virtual void Write() const
{ std::cout << "<Circle Radius=" << mCircle.radius << "/>"; }

};

struct CSquarePlus : public CShapePlus
{
Square mSquare;
virtual void Write() const
{ std::cout << "<Square Edge=" << mSquare.edge << "/>"; }

};

// inheritance

struct CCirclePlus2 : public Circle, public CShapePlus
{
virtual void Write() const
{ std::cout << "<Circle Radius=" << this->radius << "/>"; }
};

struct CSquarePlus2 : public Square, public CShapePlus
{
virtual void Write() const
{ std::cout << "<Square Edge=" << this->edge << "/>"; }
};



//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~
// main

int main()
{
Shape * shape1 = new Circle;
Shape * shape2 = new Square;

XmlWriter writer;

writer.write( shape1 ); std::cout << '\n';
writer.write( shape2 ); std::cout << '\n';


CShapePlus * shape3 = new CCirclePlus;
CShapePlus * shape4 = new CSquarePlus;
CShapePlus * shape5 = new CCirclePlus2;
CShapePlus * shape6 = new CSquarePlus2;

shape3->Write(); std::cout << '\n';
shape4->Write(); std::cout << '\n';
shape5->Write(); std::cout << '\n';
shape6->Write(); std::cout << '\n';
}

// END CODE


 
Reply With Quote
 
Goran
Guest
Posts: n/a
 
      11-17-2011
On Nov 17, 12:20*am, Chris Stankevitz <(E-Mail Removed)>
wrote:
> On Nov 16, 2:12 pm, Ian Collins <(E-Mail Removed)> wrote:
>
> > So how would a generic drawing class be expected to know this?

>
> This is basically the question I am asking this group.
>
> Below is a pseudocode answer to the question that
> *- leaves Shape, Circle, and Square untouched (good)
> *- places no drawing dependency on the shapes (good)
> *- leaves drawing code entirely in the Drawer classes (good)
> *- does not have switch or if statements (good)
> ** does use a dynamic cast (BAD)
>
> Can you (or anyone else) come up with some c++ pseudocode that
> *- leaves Shape, Circle, and Square untouched (good)
> *- places no drawing dependency on the shapes (good)
> *- leaves drawing code entirely in the Drawer classes (good)
> *- does not have switch or if statements (good)
> *- does not use a dynamic cast (good)
>
> Chris
>
> =====
>
> //----------
> struct Drawer
> {
> * virtual void Draw(Shape* shape) = 0;
>
> };
>
> //----------
> struct CircleDrawer : public Drawer
> {
> * void Draw(Shape* shape)
> * {
> * * // dynamic_cast<Circle*>(shape)
> * * // draw the circle
> * }
>
> };
>
> //----------
> struct SquareDrawer : public Drawer
> {
> * void Draw(Shape* shape)
> * {
> * * // dynamic_cast<Square*>(shape)
> * * // draw the square
> * }
>
> };
>
> //----------
> struct DrawerCreator
> {
> * virtual Drawer* GetNewDrawer() = 0;
> * virtual bool CanDraw(Shape* shape) = 0;
>
> };
>
> //----------
> struct CircleDrawerCreator : public DrawerCreator
> {
> * Drawer* GetNewDrawer() { return new CircleDrawer; }
> * bool CanDraw(Shape* shape) { return dynamic_cast<Circle*>(shape); }
>
> };
>
> //----------
> struct SqareDrawerCreator : public DrawerCreator
> {
> * Drawer* GetNewDrawer() { return new SquareDrawer; }
> * bool CanDraw(Shape* shape) { return dynamic_cast<Square*>(shape); }
>
> };
>
> //----------
> struct DrawerManager
> {
> * void RegisterDrawerCreator(DrawerCreator* dc)
> * {
> * * drawers.push_back(dc);
> * }
>
> * void Draw(Shape* shape)
> * {
> * * if (Drawer* drawer = GetDrawer(shape))
> * * {
> * * * drawer->Draw(shape);
> * * }
> * }
>
> * Drawer* GetDrawer(Shape* shape)
> * {
> * * for (int i = 0; i < drawers.size(); ++i)
> * * {
> * * * if (drawers[i]->CanDraw(shape))
> * * * {
> * * * * return drawers[i]->GetNewDrawer();
> * * * }
> * * }
>
> * * return 0;
> * }
>
> * vector<Drawer*> drawers;
>
> };
>
> //----------
> int main()
> {
> * DrawerManager drawer_manager;
>
> * drawer_manager.RegisterDrawerCreator(new CircleDrawerCreator);
> * drawer_manager.RegisterDrawerCreator(new SquareDrawerCreator);
>
> * Shape* shape1 = new Circle;
> * Shape* shape2 = new Square;
>
> * drawer_manager.Draw(shape1);
> * drawer_manager.Draw(shape2);
>
>
>
>
>
>
>
> }


What you seem to be showing here is the adapter pattern. The way you
went about it seems backwards, though. When you create your adapter
(Drawer), you should strive to do it using the correct adaptee type.
But what you did is to throw said type back to base (by passing Shape*
to DrawerManager) and reached for dynamic_cast.

So how about this:

int main()
{
Canvas c;

Circle shape1;
Square shape2;

Draw(c, CircleDrawer(shape1));
Draw(c, SquareDrawer(shape2));
}

class Canvas { /*Driving primitives here*/ };

class Drawer
{
public: virtual void Draw(Canvas& c) const = 0;
};

void Draw(Canvas& c, Drawer& d)
{
d.Draw(c);
}

// Trivial intermediary: drawer for Shapes.
class ShapeDrawer
{
const Shape& s_;
public:
ShapeDrawer(Shape& s) : s_(s) {}
};

// Implementation starts here

class CircleDrawer : public ShapeDrawer
{
public:
CircleDrawer(const Circle& s) : ShapeDrawer(c) {}
virtual void Draw(Canvas& c) const {knock yourself out}
};

class SquareDrawer {you get the picture};

IOW... __Don't__ use generic Shapes in lib B logic. __Adapt__ them as
soon as you get them (because then you should know their types, and
that's the only way to avoid casting) and use adapters. Added bonus:
adapters will let you mix your own stuff with lib A stuff.

Goran.

P.S. You didn't want dynamic_cast, but rather static_cast, there.
 
Reply With Quote
 
Goran
Guest
Posts: n/a
 
      11-17-2011
On Nov 17, 8:39*am, Werner <(E-Mail Removed)> wrote:
> I can't see why using dynamic cast is so bad. It is not as
> if you have an if/else style of programming that requires
> constant modification. dynamic_cast under your control that
> requires no modification is good (except for being a tid
> slower, perhaps).


Well, conceptually, there's no difference between if/else and
dynamic_cast (and indeed OP's first post is dynamic_cast with if/
else). dynamic_cast merely moves decision to the type itself (whereas
if/else decides on the data). IOW, with dynamic_cast, type is used as
data. And indeed, if new type introduced, conditional logic must be
changed with itand therefore open/closed principle is compromised.

Goran.
 
Reply With Quote
 
Chris Stankevitz
Guest
Posts: n/a
 
      11-17-2011
Werner,

Thank you for your reply.

> I can't see why using dynamic cast is so bad.


Perhaps it is not bad. One way or another I am going to complete this
task and I know I can complete it using dynamic_casts so I might end
up using them. I suppose my question to the group is "is it possible
to do something like this without using a dynamic_cast." The answer
to that question might be "no" -- which I'm okay with.

> You might want to remove the burden from your client though


Thank you your method is certainly cleaner than mine. I need to look
at it a bit more closely but I believe we are both doing the same
thing.

Chris
 
Reply With Quote
 
Chris Stankevitz
Guest
Posts: n/a
 
      11-17-2011

Francesco,

Thank you for your reply.

> - map the type_info of the classes to some function


This is the sort of thing I came to the group to hear. I've never
heard of type_info before, so I am going to go learn about it. Just
from the name my suspicion is that type_info is doing the same thing
as dynamic_cast under the covers, but I'm going to find out.

Thank you for providing a novel solution to my problem that satisfies
my conditions!

Chris
 
Reply With Quote
 
Chris Stankevitz
Guest
Posts: n/a
 
      11-17-2011

Goran,

On Nov 17, 5:17*am, Goran <(E-Mail Removed)> wrote:
> What you seem to be showing here is the adapter pattern. The way you
> went about it seems backwards, though.


Thank you this is exactly the kind of information I was looking for.
I'm going to go read up on Adapters. I've never heard of 'adapter
pattern'.

> But what you did is to throw said type back to base (by passing Shape*
> to DrawerManager) and reached for dynamic_cast.


I noticed this after I posted and some others pointed that out here on
the list. I understand the concept and the idea behind sending the
correct type to the drawer classes.

Thank you,

Chris
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Re: How include a large array? Edward A. Falk C Programming 1 04-04-2013 08:07 PM
if statements and case statements questions John Crichton Ruby 6 07-12-2010 06:17 PM
Prepare Statements VS Statements Vince Java 12 01-21-2008 01:18 PM
component statements within architecture statements Neil Zanella VHDL 8 10-20-2006 09:05 AM
if statements with or w/o else statements Harry George Python 6 02-23-2004 06:48 PM



Advertisments