inheritance headache....

B

bob

Hi experts,

I have a construction problem that I need to fix;


currrently the following hierarchy exists;



abstractTarget
/ \
/ \
conreteTarget1 concreteTarget2



abstractDescription (this class family can NOT be modified)
/ \
concreteDesc1 concDesc1


Now somewhere, actually EVERYWHERE in this legacy code I see
statements like this;

// semi pseudo with
abstractTarget* createTarget(abstractDescription* desc)
{

// shortcut pseudo code for dynamic cast< > ()
if ( desc is concreteDesc1)
{
return new concreteTarget1();
}
else if (desc is concreteDesc2)
{
return new concreteTarget2();
}
else
{
return 0;

}

}

Now I CANT add methods to abstractDescription, if I could I would have
a method;

class abstractDescription
{
AbstractTarget* create(AbstractDescription* desc)
{
desc->create();
}

}

and each ConcreteDescription subclass would override as needed the
create the right concreteTarget.

BUT I cant do that as the AbstractDesciption family of objects cannot
be modified in any way. Can anybody help me out?


Im trying to tease out a solution here but I always get stung with a
cast or such like. anybody out there have any ideas?


cheers

G
 
A

Alf P. Steinbach

* (e-mail address removed):
I have a construction problem that I need to fix;


currrently the following hierarchy exists;



abstractTarget
/ \
/ \
conreteTarget1 concreteTarget2



abstractDescription (this class family can NOT be modified)
/ \
concreteDesc1 concDesc1


Now somewhere, actually EVERYWHERE in this legacy code I see
statements like this;

// semi pseudo with
abstractTarget* createTarget(abstractDescription* desc)
{

// shortcut pseudo code for dynamic cast< > ()
if ( desc is concreteDesc1)
{
return new concreteTarget1();
}
else if (desc is concreteDesc2)
{
return new concreteTarget2();
}
else
{
return 0;

}

}

Now I CANT add methods to abstractDescription, if I could I would have
a method;

class abstractDescription
{
AbstractTarget* create(AbstractDescription* desc)
{
desc->create();
}

}

It's clear what you mean, something like

struct AbstractDescription
{
virtual std::auto_ptr<AbstractTarget> create() const = 0;
};

and each ConcreteDescription subclass would override as needed the
create the right concreteTarget.

BUT I cant do that as the AbstractDesciption family of objects cannot
be modified in any way. Can anybody help me out?


Im trying to tease out a solution here but I always get stung with a
cast or such like. anybody out there have any ideas?

E.g. install factory functions in a std::map using typeinfo (from typeid
operator) as key. Then given an AbstractDescription, apply typid, look
up pointer in map, call resulting factory function. Only tricky thing
is to get that map properly initialized.

Cheers, & hth.,

- Alf
 
T

tragomaskhalos

Hi experts,

I have a construction problem that I need to fix;

Now somewhere, actually EVERYWHERE in this legacy  code I see
statements like this;

// semi pseudo with
abstractTarget* createTarget(abstractDescription* desc)
{
// shortcut pseudo code for dynamic cast< > ()
   if ( desc is concreteDesc1)
   {
     return new concreteTarget1();
   }
   else if (desc is concreteDesc2)
   {
     return new concreteTarget2();
   }
   else
   {
     return 0;
   }
}

Now I CANT add methods to abstractDescription, if I could I would have
a method;

It's not clear from your post if it's the *same* type-switching
code being used to create concrete objects, or different
flavours of type-switching code doing a variety of things.
If it's the former, the best you can do is refactor to a
single createTarget method - and, ugly as it is, I don't
see how to significantly improve what you have within the
constraints cited (assuming there are indeed only 2 subclasses
involved - if there are rather more, there are better ways
than a succession of if statements).
 
S

Sean Hunt

Hi experts,

I have a construction problem that I need to fix;

currrently the following hierarchy exists;

abstractTarget
/ \
/ \
conreteTarget1 concreteTarget2

abstractDescription (this class family can NOT be modified)
/ \
concreteDesc1 concDesc1

Now somewhere, actually EVERYWHERE in this legacy code I see
statements like this;

// semi pseudo with
abstractTarget* createTarget(abstractDescription* desc)
{

// shortcut pseudo code for dynamic cast< > ()
if ( desc is concreteDesc1)

Don't use dynamic_cast to check whether a class is of a specific type.
Only use it if you need the actual dervied object - otherwise, use
typeid:

if (typeid(desc) == typeid(concreteDesc1))

typeid will return an std::type_info object corresponding to the most
derived type. Remember to include said:
{
return new concreteTarget1();
}
else if (desc is concreteDesc2)
{
return new concreteTarget2();
}
else
{
return 0;

}

}

Now I CANT add methods to abstractDescription, if I could I would have
a method;

class abstractDescription
{
AbstractTarget* create(AbstractDescription* desc)
{
desc->create();
}

}

and each ConcreteDescription subclass would override as needed the
create the right concreteTarget.

BUT I cant do that as the AbstractDesciption family of objects cannot
be modified in any way. Can anybody help me out?

As far as I know, there's no way to properly reproduce the virtual
call mechanism. However, a map of typeids will work:

bool operator <
 
S

Sean Hunt

Hi experts,

I have a construction problem that I need to fix;

currrently the following hierarchy exists;

abstractTarget
/ \
/ \
conreteTarget1 concreteTarget2

abstractDescription (this class family can NOT be modified)
/ \
concreteDesc1 concDesc1

Now somewhere, actually EVERYWHERE in this legacy code I see
statements like this;

// semi pseudo with
abstractTarget* createTarget(abstractDescription* desc)
{

// shortcut pseudo code for dynamic cast< > ()

You can compare type_id
if ( desc is concreteDesc1)
{
return new concreteTarget1();
}
else if (desc is concreteDesc2)
{
return new concreteTarget2();
}
else
{
return 0;

}

}

Now I CANT add methods to abstractDescription, if I could I would have
a method;

class abstractDescription
{
AbstractTarget* create(AbstractDescription* desc)
{
desc->create();
}

}

and each ConcreteDescription subclass would override as needed the
create the right concreteTarget.

BUT I cant do that as the AbstractDesciption family of objects cannot
be modified in any way. Can anybody help me out?

Use a map of typeid:

// You will need every target to have a static create() method.

#include <typeinfo>

// Comparison operator
bool compare (const std::type_info & lhs, const std::type_info & rhs)
{
return lhs.before(rhs);
}

//Map
std::map <const std::type_info &, abstractTarget (*) ()> type_map;

void initialize_type_map()
{
type_map[typeid(concreteDesc1)] = &concreteTarget1::create;
type_map[typeid(concreteDesc2)] = &concreteTarget2::create;
}

Then you can just write:
absractTarget* target = mymap[typeid(desc)]();

You could also have a wrapper class to hide all of this stuff.
Im trying to tease out a solution here but I always get stung with a
cast or such like. anybody out there have any ideas?

cheers

G

NOTE: The previous post was made incomplete. Sorry about that.
 
B

bob

Thanks for the replies. To answer some questions/remarks;

1) There are a good few concreteTarget and concreteDescription
subclasses and there will be lots more added in the months to come.
This means the

if ( desc is concreteDesc1)
{
return new concreteTarget1();
}
else if (desc is concreteDesc2)
{
return new concreteTarget2();
}
else
{
return 0;
}

control statements will get out of control.


"It's not clear from your post if it's the *same* type-switching
code being used to create concrete objects, or different
flavours of type-switching code doing a variety of things."

Its actually both. Throughout the code, ConcreteTarget objects are
created using the logic above and also to perform various method
invocations. For the moment, I'm just concerned about the creation of
the obejcts. I think the smartest solution so far is perhaps that
provide by Sean Hunt (thanks Sean). I could wrap the code and put it
into a factory object.

If anybody has anything to add, I'm all ears. I'm thinking there HAS
to be a smarter way. I really want to avoid any sort of lookup or if/
else if logic as I've been reading up on my OO design books. Martin
Fowlers refactoring book doesn't cover it. Gamma et Al don't deal with
this scenario in Design Patterns and I didn't have time to look at my
copy of "Working with Legacy Code" last night as it was 11pm and I
wanted a beer! :) :)

Anyways, thanks for the inputs. If anybody has any more wicked ideas
on this conundrum, I'd be DELIGHTED to hear.


Thanks again,

G
Hi experts,
I have a construction problem that I need to fix;
currrently the following hierarchy exists;
       abstractTarget
       /           \
      /             \
 conreteTarget1  concreteTarget2
       abstractDescription (this class family can NOT be modified)
         /          \
   concreteDesc1  concDesc1
Now somewhere, actually EVERYWHERE in this legacy  code I see
statements like this;
// semi pseudo with
abstractTarget* createTarget(abstractDescription* desc)
{
// shortcut pseudo code for dynamic cast< > ()

You can compare type_id




   if ( desc is concreteDesc1)
   {
     return new concreteTarget1();
   }
   else if (desc is concreteDesc2)
   {
     return new concreteTarget2();
   }
   else
   {
     return 0;
   }

Now I CANT add methods to abstractDescription, if I could I would have
a method;
class abstractDescription
{
  AbstractTarget* create(AbstractDescription* desc)
  {
      desc->create();
  }

and each ConcreteDescription subclass would override as needed the
create the right concreteTarget.
BUT I cant do that as the AbstractDesciption family of objects cannot
be modified in any way. Can anybody help me out?

Use a map of typeid:

// You will need every target to have a static create() method.

#include <typeinfo>

// Comparison operator
bool compare (const std::type_info & lhs, const std::type_info & rhs)
{
  return lhs.before(rhs);

}

//Map
std::map <const std::type_info &, abstractTarget (*) ()> type_map;

void initialize_type_map()
{
  type_map[typeid(concreteDesc1)] = &concreteTarget1::create;
  type_map[typeid(concreteDesc2)] = &concreteTarget2::create;

}

Then you can just write:
absractTarget* target = mymap[typeid(desc)]();

You could also have a wrapper class to hide all of this stuff.
Im trying to tease out a solution here but I always get stung with a
cast or such like. anybody out there have any ideas?

G

NOTE: The previous post was made incomplete. Sorry about that.- Hide quoted text -

- Show quoted text -- Hide quoted text -

- Show quoted text -
 
T

Triple-DES

//Map
std::map <const std::type_info &, abstractTarget (*) ()> type_map;

void initialize_type_map()
{
  type_map[typeid(concreteDesc1)] = &concreteTarget1::create;
  type_map[typeid(concreteDesc2)] = &concreteTarget2::create;

}

Remember that you can't have containers of references. To make the
type_map work here you need it to operate on std::type_info pointers
(with a predicate that compares the pointers in terms of the "before"
member function), or write a small wrapper class for std::type_info.
 
J

James Kanze

* (e-mail address removed):
It's clear what you mean, something like
struct AbstractDescription
{
virtual std::auto_ptr<AbstractTarget> create() const = 0;
};

How is it so clear? Normally, auto_ptr suggests that the caller
will be responsible for deleting the object. And most of the
time I've seen such a pattern used, this simply isn't the
case---the object registers itself in its constructor for some
sort of external events, and deletes itself when the appropriate
event arises.
 
J

James Kanze

[...]
Don't use dynamic_cast to check whether a class is of a
specific type. Only use it if you need the actual dervied
object - otherwise, use typeid:
if (typeid(desc) == typeid(concreteDesc1))
typeid will return an std::type_info object corresponding to
the most derived type. Remember to include <typeinfo> before
using typeid!

The semantics are different. Use typeid if you want to know the
exact most derived type, dynamic_cast if you only want to know
whether the object is a, or not. (In this case, I suspect that
the desired semantics are in fact those of typeid, and not
dynamic_cast.)

[...]
As far as I know, there's no way to properly reproduce the virtual
call mechanism. However, a map of typeids will work:
bool operator <

Two gotcha's for mapping type_info's to anything:

1. A type_info is not copiable, so you can't put them in a
container. You need to use type_info const*.

2. type_info, per se, doesn't support operator<. You need to
provide your own, based on type_info::before(). (Hopefully,
the next version of the standard will correct this, and also
provide functions for == and getting a hash code of a
type_info.)

For these reasons, I tend to use a wrapper class, something
like:

class Type
{
public:
/* implicit */ Type( std::type_info const& type )
: myType( &type )
{
}

bool operator<( Type const& other ) const
{
return myType->before( *other.myType ) ;
}

private:
std::type_info const*myType ;
} ;

typedef std::map< Type, whatever >

--
James Kanze (GABI Software) email:[email protected]
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
TypeMap ;
 
A

Alf P. Steinbach

* James Kanze:
How is it so clear? Normally, auto_ptr suggests that the caller
will be responsible for deleting the object.

Yes, that was the case.

And most of the
time I've seen such a pattern used, this simply isn't the
case---the object registers itself in its constructor for some
sort of external events, and deletes itself when the appropriate
event arises.

And that wasn't the case.


Cheers, & hth.,

- Alf
 
P

Pete Becker

2. type_info, per se, doesn't support operator<. You need to
provide your own, based on type_info::before(). (Hopefully,
the next version of the standard will correct this, and also
provide functions for == and getting a hash code of a
type_info.)

It's not going to happen if nobody proposes it. (that's a hint...)
 
B

bob

How is it so clear?  Normally, auto_ptr suggests that the caller
will be responsible for deleting the object.  And most of the
time I've seen such a pattern used, this simply isn't the
case---the object registers itself in its constructor for some
sort of external events, and deletes itself when the appropriate
event arises.

--
James Kanze (GABI Software)             email:[email protected]
Conseils en informatique orientée objet/
                   Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34

this definitely has grabbed my attention as a potentially more elegant
solution. Does anybody have any docs/examples of this in action... I
would be delighted to read more. I was already thinking about this in
parallel... though am missing the details. At this stage I am very
much interested in the most elegant solution....performance is not an
issue (if that can ever be said when writing "elegant" code).


BTW Nobody needs to worry about auto_ptrs or memory mgmt here, Thats
all covered, the sample code/scenario is generalised/simplified.


Thanks *very* much for the input, I find the discussion very
interesting and revealing.

G
 
J

James Kanze

this definitely has grabbed my attention as a potentially more elegant
solution. Does anybody have any docs/examples of this in action... I
would be delighted to read more. I was already thinking about this in
parallel... though am missing the details. At this stage I am very
much interested in the most elegant solution....performance is not an
issue (if that can ever be said when writing "elegant" code).

Which is the elegant solution, what Alf suggested, or what I
suggested? Or perhaps better stated, what is the role of the
class in the application: my solution is generally applicable to
entity classes, but I use Alf's (or something similar, with my
own reference counted pointers) when I'm dealing with agents in
legacy applications (where I can't use the Boehm collector).

Anyway, my solution very much depends on how you manage the
objects in general. One typical case (at least in the type of
servers I work on) is where the objects are addresses from the
outside via some external name (DN or other); the constructor
registers the object in a map (std::map< DN, AbstractTarget* >)
so that the request handler can find it. Delete requests (like
all other requests) are forwarded to the object, which does a
delete this. The destructor removes the object from from the
map.

In another frequent idiom, the object will first be passed to a
transaction manager, which is responsible for deleting it until
the commit phase of the transaction, at which time it is
enrolled in the final registry.

In other cases, the object will register itself with an event
hander for some sort of external events; certain events will
tell the object that it is no longer needed, in which case, it
will delete itself (also deenrolling in the destructor).

In my experience, such scenarios represent the majority of
polymorphic objects.
 
J

James Kanze

On 2008-02-01 07:57:58 -0500, James Kanze <[email protected]> said:
It's not going to happen if nobody proposes it. (that's a hint...)

If it's not too late, I'll write one up this week-end. (I
really wanted to contribute a lot more, but I've had problems
finding the time. So when something small like this presents
itself... At least I can do something.)
 
P

Pete Becker

If it's not too late, I'll write one up this week-end.

It never hurts to ask.
(I
really wanted to contribute a lot more, but I've had problems
finding the time. So when something small like this presents
itself... At least I can do something.)

<g>
 
A

Alf P. Steinbach

* James Kanze:
How do you know? There was nothing in the original posting to
suggest it, and it's rather the exception, and not the rule.

Now somewhere, actually EVERYWHERE in this legacy code I see
statements like this;

// semi pseudo with
abstractTarget* createTarget(abstractDescription* desc)
{

// shortcut pseudo code for dynamic cast< > ()
if ( desc is concreteDesc1)
{
return new concreteTarget1();
}
else if (desc is concreteDesc2)
{
return new concreteTarget2();
}
else
{
return 0;
}
}

Notice that no information about the created objects is retained.

Hence, as I figure it, the caller has the responsibility for destroying
them.


Cheers, & hth.,

- Alf
 
G

Grizlyk

Now somewhere, actually EVERYWHERE in this legacy  code I see
statements like this;

// semi pseudo with
abstractTarget* createTarget(abstractDescription* desc)
{

// shortcut pseudo code for dynamic cast< > ()
   if ( desc is concreteDesc1)
   {
     return new concreteTarget1();
   }
   else if (desc is concreteDesc2)

you not need "else" after "return"
   {
     return new concreteTarget2();
   }
   else
   {
     return 0;

   }
}

Not sure about "actually EVERYWHERE", but the job is well-known design
pattern called as Abstract Factory.

The Factory returns concrete set of objects and knowledge about
correct type of returned objects is responsibility of the Factory,
encapsulated by the Factory, why "actually EVERYWHERE"?

Abstract Factory often used as link-time compatible part of program,
so implementation of the Factory is often runtime template (looks like
trivial case of well-known design pattern called as Template Method) -
removing implementation of interface of abstract base class into
derived classes.

Abstract Factory declare interface of creators:

class Abstract_Factory
{
public:

virtual
abstractTarget*
Abstract_Factory::createTarget
(abstractDescription* desc)
=0;
};

Concrete Factory, derived from Abstract Factory, encapsulates concrete
set of returned objects:

namespace N1feb2008{
class Concrete_Factory: public Abstract_Factory
{
public:
abstractTarget*
concrete_Factory::createTarget
(abstractDescription* desc)
{
if ( desc is concreteDesc1)
return new concreteTarget1;

if (desc is concreteDesc2)
return new concreteTarget2;

return 0;
}
};}

namespace N2feb2008{
template
<
class Told_Factory=N1feb2008::Concrete_Factory
class Concrete_Factory: public Abstract_Factory
{
Told_Factory old;

public:
abstractTarget*
concrete_Factory::createTarget
(abstractDescription* desc)
{
if ( desc is concreteDesc3)
return new concreteTarget3;

return old.createTarget(desc);
}
};}
Now I CANT add methods to abstractDescription,
if I could I would have a method;

class abstractDescription
{
  AbstractTarget* create(AbstractDescription* desc)
  {
      desc->create();
  }
}

and each ConcreteDescription subclass would override
as needed the create the right concreteTarget.

BUT I cant do that as the AbstractDesciption family
of objects cannot be modified in any way.

Not sure what does it mean "cannot be modified in any way". There is
well-known design pattern Adapter, that can adapt in fact everything
to everywhere.

In the case, Adapter will add to abstractDescription new method for
new "create" message of new requested interface (class
Adapted_Description).

//compile time template
namespace Nctt
{
//types
//they encapsulate links
//between Target and Description

template<class Tadapted>
struct Tlink;

//
template<>struct
Tlink<concreteDescription1>
{ typedef concreteTarget1 Target; };

//
template<>struct
Tlink<concreteDescription2>
{ typedef concreteTarget2 Target; };

//
template<>struct
Tlink<concreteDescription3>
{ typedef concreteTarget3 Target; };

//end of types

//adapter
template<class Tadapted, class Towner=Tadapted>
class Adapted_Description: public abstractDescription
{
public:
typedef
typename Tlink<Tadapted>::Target
Target;

protected:
Towner adapted;

//new interface
public:
Target* create()
{ return new Target; }

Adapted_Description();
Adapted_Description(const Towner& ad):
adapted(ad);

//old interface
public:

for each
abstractDescription::method
{ adapted.abstractDescription::method; }

};

//namespace Nctt
}

//run time template
namespace Nrtt
{

class Adapted_Description: public abstractDescription
{
//new interface
public:
virtual
AbstractTarget*
create()
=0;
};

template<class Tadapted, class Towner=Tadapted>
class concrete_Description: public Adapted_Description
{
public:
typedef
typename Nctt::Adapted_Description
<Tadapted, Towner>
Nctt_Adapted_Description;

typedef
typename
Nctt_Adapted_Description::Target
Target;

protected:
Nctt_Adapted_Description
adapted;

//Ncct::Adapted_Description interface

for each
Nctt_Adapted_Description ctor
{ concrete_Description():adapted(); }
/*
concrete_Description()
adapted();
concrete_Description(const Towner& ad):
adapted(ad);
*/

/*
//new interface (part of Adapted_Description interface)
public:
Target*
create()
{ return adapted.create(); }
*/

//old interface
public:

for each
Adapted_Description::method
{ adapted.Ncct_Adapted_Description::method; }

};

//namespace Nrtt
}

This can be compiled

void foo()
{
Nctt::Adapted_Description
<concreteDescription1>
tmp;

Nrtt::concrete_Description
<concreteDescription1>
tmp2;

tmp.create();
tmp2.create();
}

declare and define all

class abstractDescription{};
class AbstractTarget{};

class concreteDescription1:
public abstractDescription {};
class concreteDescription2:
public abstractDescription {};
class concreteDescription3:
public abstractDescription {};

class concreteTarget1:
public AbstractTarget {};
class concreteTarget2:
public AbstractTarget {};
class concreteTarget3:
public AbstractTarget {};

and comment "for each" sections and not-defined ctors

Maksim A. Polyanin
http://grizlyk1.narod.ru/cpp_new
 
A

Alf P. Steinbach

* James Kanze:
* James Kanze:
[...]
How do you know? There was nothing in the original posting to
suggest it, and it's rather the exception, and not the rule.
Now somewhere, actually EVERYWHERE in this legacy code I see
statements like this;
// semi pseudo with
abstractTarget* createTarget(abstractDescription* desc)
{
// shortcut pseudo code for dynamic cast< > ()
if ( desc is concreteDesc1)
{
return new concreteTarget1();
}
else if (desc is concreteDesc2)
{
return new concreteTarget2();
}
else
{
return 0;
}
}
Notice that no information about the created objects is retained.
Hence, as I figure it, the caller has the responsibility for destroying
them.

In other words, since no information is given, you suppose the
exceptional case, rather than the usual.

No. First, there's lots of information in the example above. Second,
what's "usual" for you is, I suspect, rather different than the norm in
industry, especially for old legacy code; a better term might be
"ideal", that you're wondering why I'm assuming the code isn't ideal...

Can you really imagine the programmer who created the above, doing
registration for events in e.g. concreteTarget1's constructor?

I can't -- but then, if the OP states otherwise (which would be
inconsistent with comments else-thread), I'd have to revise my opinion.


Cheers, & hth.,

- Alf
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,774
Messages
2,569,598
Members
45,152
Latest member
LorettaGur
Top