a problem with poliporphism

[KPB]

Why not? You would've seen that your function doesn't compile. When
something doesn't compile, that's not GOOD DESIGN, right?




That would be a good start since a pointer to your base class is calling
these functions.

KPB

One more thing. You state that a Crime and an Emergency is an event.
Ok... that makes good sense from a ISA point of view.

But think about this further: Is a Crime an Emergency? Perhaps your
hierarchy can be Event --> Emergency --> Crime? I don't know. I'm just
trying to get you to think about the DESIGN better.

KPB

 

[Nafai]

Let's focus on this function:

Do you think it is a good a idea to do this:
void doSomethingWithEvents(Event* pE)
{
Emergency* pEmergency;
switch(pE->kind) {
case Event::Emergency :
pEmergency=dynamic_cast<Emergency*>(pE);
callAmbulance(pEmergency->where(), pEmergency->when()); break;
... // and so on
}
}

 

[KPB]

Do you think it is a good a idea to do this:
void doSomethingWithEvents(Event* pE)
{
Emergency* pEmergency;
switch(pE->kind) {
case Event::Emergency :
pEmergency=dynamic_cast<Emergency*>(pE);
callAmbulance(pEmergency->where(), pEmergency->when()); break;
... // and so on
}
}

No. That leads me to another point on your classes. What's the point in
having polimorphism if the specific kind of event is stored as some
internal value of the Event class? Every time you add a new class, you
have to update your Event class to deal with a new "kind" enumeration.

So, with your current design, when you derive a class from Event, you
also have to modify the Event class. Not good design.

Ditch the enum list in Event.

KPB

 

[Jeff Flinn]

I am going to give a more detailed example:

// I don't type the constructors.

class Event {
private:
int time;
EventType kind;
public:
enum EventType { Event1, Event2,...,Emergency,Crime};
int when() {return time; }
EventType what() { return kind; }
}

class Emergency : public Event {
private:
int area;
public:
int where() {return area;}
};

class Crime : public Event {
private:
string person;
int aCrime;.
public:
string who() { return person; }
int whatHeDid() { return aCrime; }
};

void doSomethingWithEvents(Event* pE)
{
switch(pE->kind) {
case Event::Emergency :
callAmbulance(pE->where(),pE->when()); break;
case Event::Crime:
blame(pE->who(),pE->whatHeDid(),pE->when()); break;
case Event::Event1 : doSomethingRelatedToEvent1(pE->when());
break; ...
}
}

int main() {
list<Event*> lst;
... // insert somehow Events in lst
Event* pEvt = lst.first();
lst.pop_first();
doSomethingWithEvent(pEvt);
...
}

My question is: which is the best way to do that? Is it OK like
before?

It's not 'OK' as it was not 'OK' before.

All of my previous comments apply. The only difference I see here is you've
renamed 'p' to doSomethingWithEvents. What was wrong with the approach I
previously suggested?

I can not do your job for you. I can only suggest that you do some reading,
such as "Design Patterns" by GoF, and "Accelerated C++" by K & M.

Jeff

 

[Jonathan Mcdougall]

I am going to give a more detailed example:

// I don't type the constructors.

class Event {
private:
int time;
EventType kind;
public:
enum EventType { Event1, Event2,...,Emergency,Crime};
int when() {return time; }
EventType what() { return kind; }

Event is meant to be used as a base class and does
not have a virtual destructor nor virtual
functions. What kind of base class is that?

}

class Emergency : public Event {
private:
int area;
public:
int where() {return area;}
};

class Crime : public Event {
private:
string person;
int aCrime;.
public:
string who() { return person; }
int whatHeDid() { return aCrime; }
};


void doSomethingWithEvents(Event* pE)
{
switch(pE->kind) {
case Event::Emergency : callAmbulance(pE->where(),pE->when());
break;
case Event::Crime: blame(pE->who(),pE->whatHeDid(),pE->when());
break;
case Event::Event1 : doSomethingRelatedToEvent1(pE->when()); break;
...
}
}

That is not recommended. You seem not to
understand the use of polymorphism and public
inheritance.

Public inheritance has many uses, but in your
case, it is a simple case of interface<->behavior.
The base class specifies an interface (with
[pure] virtual functions) and derived classes
specify the behavior.

class Event
{
public:
virtual ~Event();

virtual void process() = 0; // that's the
// interface
};

class Emergency : public Event
{
private:
void call_ambulance();

public:
virtual void process() // that's a behavior
{
call_ambulance();
}
};

class Crime : public Event
{
private:
void blame();

public:
virtual void process()
{
blame(); // that's another behavior
}
};


typedef std::sector<Event*> Events;

void f(Events &events)
{
for (Events::iterator itor=events.begin();
itor!=events.end();
++itor)
{
Event *e = *itor;
e->process(); // calls the correct
// process()
}
}


Jonathan

 

[Nafai]

KPB escribió:

No. That leads me to another point on your classes. What's the point in
having polimorphism if the specific kind of event is stored as some
internal value of the Event class? Every time you add a new class, you
have to update your Event class to deal with a new "kind" enumeration.

So, with your current design, when you derive a class from Event, you
also have to modify the Event class. Not good design.

Ditch the enum list in Event.

KPB

OK. So what desing would be the best in your opinion? How would you
solve that problem?

 

[KPB]

KPB escribió:


OK. So what desing would be the best in your opinion? How would you
solve that problem?

I've told you quite a bit about what I'd do.

I'm sorry but I think you need to study the topic of inheritence a
little better.

KPB

 

[Nafai]

KPB escribió:

I've told you quite a bit about what I'd do.

I'm sorry but I think you need to study the topic of inheritence a
little better.

KPB

Just tell me an outline of your solution. I know what it is polymorphism
and what is inheritance. Please, just tell me your solution. If you have it.

 

[Jeff Flinn]

KPB escribió:
....

Just tell me an outline of your solution. I know what it is
polymorphism and what is inheritance. Please, just tell me your
solution. If you have it.

You've been told a few times by a few people already. Give it your best
shot, post your try here and you'll get some help.

Jeff

 

[Nafai]

I am going to give a more detailed example:

// I don't type the constructors.

class Event {
private:
int time;
EventType kind;
public:
enum EventType { Event1, Event2,...,Emergency,Crime};
int when() {return time; }
EventType what() { return kind; }

Event is meant to be used as a base class and does not have a virtual
destructor nor virtual functions. What kind of base class is that?

}

class Emergency : public Event {
private:
int area;
public:
int where() {return area;}
};

class Crime : public Event {
private:
string person;
int aCrime;.
public:
string who() { return person; }
int whatHeDid() { return aCrime; }
};


void doSomethingWithEvents(Event* pE)
{
switch(pE->kind) {
case Event::Emergency : callAmbulance(pE->where(),pE->when());
break;
case Event::Crime: blame(pE->who(),pE->whatHeDid(),pE->when());
break;
case Event::Event1 : doSomethingRelatedToEvent1(pE->when());
break;
...
}
}

That is not recommended. You seem not to understand the use of
polymorphism and public inheritance.

Public inheritance has many uses, but in your case, it is a simple case of
interface<->behavior. The base class specifies an interface (with [pure]
virtual functions) and derived classes specify the behavior.

class Event
{
public:
virtual ~Event();

virtual void process() = 0; // that's the
// interface
};

class Emergency : public Event
{
private:
void call_ambulance();

public:
virtual void process() // that's a behavior
{
call_ambulance();
}
};

class Crime : public Event
{
private:
void blame();

public:
virtual void process()
{
blame(); // that's another behavior
}
};


typedef std::sector<Event*> Events;

void f(Events &events)
{
for (Events::iterator itor=events.begin();
itor!=events.end();
++itor)
{
Event *e = *itor;
e->process(); // calls the correct
// process()
}
}


Jonathan

That's OK, but I can't do that. If I could of course I would have. The
"process()" is done by another class. That is: I DO NEED to take "area",
"person" etc with where(), who() etc. There's no way to change this.

 

[Jonathan Mcdougall]

That's OK, but I can't do that. If I could of course I would have. The
"process()" is done by another class. That is: I DO NEED to take "area",
"person" etc with where(), who() etc. There's no way to change this.

If you can't have virtual functions, you need to
emulate them either by using an id-switch (as you
did) or a type switch with dynamic_cast.

void f(Event *event)
{
if ( Emergency *e =
dynamic_cast<Emergency*>(event) )
{
}
else if ( Crime *c =
dynamic_cast<Crime*>(event) )
{
}
}

But that means f() must know about all the
subtypes and must test from down to top of the
hierarchy. That's a pain and it's ugly, but you
may have no choice.


Jonathan

 

[Mike Hewson]

Nafai wrote:

[snip]

What is poliporphism?

Pinch your nose and try saying "polyMorphism". Whaddya get?

For me, alas, a fart.
Seriously, it's a cross between a parrot widget and a dolphin thingy.

 

[Howard]

That's OK, but I can't do that. If I could of course I would have. The
"process()" is done by another class. That is: I DO NEED to take "area",
"person" etc with where(), who() etc. There's no way to change this.

How about passing a pointer to the instance of the class that will handle
the process() call? Or, perhaps better, make the pointer a member of the
Event class, and initialize it when creating the Event objects? Like
this...


// forward declared, so pointer can be used here
class ProcessClass;

class Event
{
ProcessClass* pProcessor;

public:
Event( ProcessClass* theProcessor ) : pProcessor(theProcessor);

virtual ~Event();

virtual void process() = 0; // that's the
// interface
};

class Emergency : public Event
{
public:
virtual void process() // that's a behavior
{
pProcessor->call_ambulance(where(),when());
}
};

class Crime : public Event
{
public:
virtual void process()
{
pProcessor->blame(who(),whatHeDid(),when()); // that's
another behavior
}
};


typedef std::sector<Event*> Events;

void f(Events &events)
{
for (Events::iterator itor=events.begin();
itor!=events.end();
++itor)
{
Event *e = *itor;
e->process(); // calls the correct
// process()
}
}

I haven't compiled this, just modified what Jonathon wrote, and I haven't
added back in the functions who(), where(), etc., but you can do that
yourself. Does this concept make sense now?

-Howard