🔒 Locked
Design Pattern: Variant
Advertisement
Original Post
Comments for the article Design Pattern: Variant
Thanx, that should come in handy. I''ve been using the MessageHandler class ever since I read Game Programming Tricks of the Trade and its come in useful several times. This should make it even better, cheers
pan narrans | My Website | Study + Hard Work + Loud Profanity = Good Code
pan narrans | My Website | Study + Hard Work + Loud Profanity = Good Code
Great article!
The thing I didn''t like though was the bDelete parameter. If the caller knows that the message should be deleted when done with it, he can delete the message himself, when the call returns. It is a lot cleaner. It doesn''t have to be a two-liner, you can actually get away with:
#include <iostream>
class A {};
void f(A* a) {
std::cout << "f()\n";
}
int main() {
f(&A());
}
It surprised me that this work even when f doesn''t declare parameter a to be const, as in void f(const A* a).
I think that in this case references are better, so it is more evident that no delete is necessary. You can take the address of the object inside the function and dynamic_cast it instead of the original reference. The code then is:
#include <iostream>
struct A {};
void f(A& a) {
std::cout << "f()\n";
}
int main() {
f(A());
}
I wonder why, instead of having the chain of if-else-if over the return of GetMessageID(), you don''t do if-else-if trying to dynamic_cast to the apropiate type each time. You save FMessageID.
Bruno
The thing I didn''t like though was the bDelete parameter. If the caller knows that the message should be deleted when done with it, he can delete the message himself, when the call returns. It is a lot cleaner. It doesn''t have to be a two-liner, you can actually get away with:
#include <iostream>
class A {};
void f(A* a) {
std::cout << "f()\n";
}
int main() {
f(&A());
}
It surprised me that this work even when f doesn''t declare parameter a to be const, as in void f(const A* a).
I think that in this case references are better, so it is more evident that no delete is necessary. You can take the address of the object inside the function and dynamic_cast it instead of the original reference. The code then is:
#include <iostream>
struct A {};
void f(A& a) {
std::cout << "f()\n";
}
int main() {
f(A());
}
I wonder why, instead of having the chain of if-else-if over the return of GetMessageID(), you don''t do if-else-if trying to dynamic_cast to the apropiate type each time. You save FMessageID.
Bruno
Actually, a short while after that article was written, I switched to the following:
I had toyed with "const MGeneric&" for HandleMessage, but as some types of message might have fields for return values, that was unsuitable.
If there is a derived type of MGeneric called (for sake of example and whimsy) MSpecific, there are a couple ways to go about getting at the information:
-OR-
Personally, I don't like the first way, but that's just me. I don't like try/catch being part of the way normal business is carried out. I much prefer the "safe" dynamic_cast of pointers. The worst thing that can happen is it returns NULL.
Switching to a reference gets rid of the bDelete parameter, and still allows constructs like this:
I did like the idea put forth by Br1, using &MGeneric(MSGID_Something()) to pass to the handler, but a lagging issue still remains: NULL pointers. As long as the type is MGeneric*, you can always pass NULL, whereas you cannot with MGeneric&.
as for the reason for the large if-else-if chain, not every messageid will have its own class. MGeneric, for example, can be used as is for a number of different messages, namely messages that don't require any extra information(like "quit").
bool IMessageHandler::HandleMessage(MGeneric& Message);
bool IMessageHandler::OnMessage(MGeneric& pMessage);
I had toyed with "const MGeneric&" for HandleMessage, but as some types of message might have fields for return values, that was unsuitable.
If there is a derived type of MGeneric called (for sake of example and whimsy) MSpecific, there are a couple ways to go about getting at the information:
bool CDerivedMessageHandler::OnMessage(MGeneric& Message)
{
.
.
.
try
{
//cast to MSpecific
MSpecific& Specific=dynamic_cast<MSpecific&>(Message);
//do something with Specific here
}
catch(...)
{
//bad cast
}
.
.
.
}
-OR-
bool CDerivedMessageHandler::OnMessage(MGeneric& Message)
{
.
.
.
//cast to MSpecific
MSpecific* pSpecific=dynamic_cast<MSpecific*>(&Message);
if(pSpecific)
{
//do something with pSpecific here
}
.
.
.
}
Personally, I don't like the first way, but that's just me. I don't like try/catch being part of the way normal business is carried out. I much prefer the "safe" dynamic_cast of pointers. The worst thing that can happen is it returns NULL.
Switching to a reference gets rid of the bDelete parameter, and still allows constructs like this:
MyMessageHandler.HandleMessage(MGeneric(MSGID_Overflow()));
I did like the idea put forth by Br1, using &MGeneric(MSGID_Something()) to pass to the handler, but a lagging issue still remains: NULL pointers. As long as the type is MGeneric*, you can always pass NULL, whereas you cannot with MGeneric&.
as for the reason for the large if-else-if chain, not every messageid will have its own class. MGeneric, for example, can be used as is for a number of different messages, namely messages that don't require any extra information(like "quit").
My guess is that that''s psuedo-C++ used to condense the syntax of static member declaration, because it''s almost a necessity that such things be scattered into the implementation file.
Would be a nice feature as written, though
Would be a nice feature as written, though
How about:
struct MGeneric {
virtual ~MGeneric() {}; // so that dynamic_cast works
};
struct MQuit : MGeneric {
};
struct MMouse : MGeneric {
int x, y;
MMouse(int x,int y) : x(x), y(y) {}
};
bool CDerivedMessageHandler::OnMessage(MGeneric& message)
{
if( dynamic_cast<MQuit*>(&message) ) {
...
} else if( MMouse* m = dynamic_cast<MMouse*>(&message) ) {
...
} else {
...
}
}
Edit(TANSTAAFL): added < > & to aid readability.
struct MGeneric {
virtual ~MGeneric() {}; // so that dynamic_cast works
};
struct MQuit : MGeneric {
};
struct MMouse : MGeneric {
int x, y;
MMouse(int x,int y) : x(x), y(y) {}
};
bool CDerivedMessageHandler::OnMessage(MGeneric& message)
{
if( dynamic_cast<MQuit*>(&message) ) {
...
} else if( MMouse* m = dynamic_cast<MMouse*>(&message) ) {
...
} else {
...
}
}
Edit(TANSTAAFL): added < > & to aid readability.
I was working on implementing somthing like this a long time ago,
unfortunetly there is one thing that doesnt sit well with me, and thats the use of non constant ID''s
for instance doing this...
#define WindowPaint 1000
you can do...
switch(event)
{
case WindowPaint:
//code
break;
}
but like in your example...
static WindowPaint=NextID();
you would have to do...
if(event==Class.WindowPaint)
{
}
else if(...)
{
}
which is alot more coding work and harder to follow when looking through code, if you ask me... so ive been trying to find a way to have compile time defenition of Message IDs but i havent yet found a way to do it =/
Raymond Jacobs,
www.EDIGames.com
www.EtherealDarkness.com
unfortunetly there is one thing that doesnt sit well with me, and thats the use of non constant ID''s
for instance doing this...
#define WindowPaint 1000
you can do...
switch(event)
{
case WindowPaint:
//code
break;
}
but like in your example...
static WindowPaint=NextID();
you would have to do...
if(event==Class.WindowPaint)
{
}
else if(...)
{
}
which is alot more coding work and harder to follow when looking through code, if you ask me... so ive been trying to find a way to have compile time defenition of Message IDs but i havent yet found a way to do it =/
Raymond Jacobs,
www.EDIGames.com
www.EtherealDarkness.com
That''s a workable solution as well, but as I was looking at it, I thought about the following situation:
If an instance of CDerivedMessageHandler gets an MTypeB message, then as it is currently written, it will handle the message as though it were an MTypeA, because MTypeA is a base class of MTypeB.
But that''s not necessarily a bad thing. Maybe we rewrite the OnMessage handler like so:
For a relatively small number of messages, this isn''t too bad. In this case a MTypeB would be handled both as a MTypeA AND as an MTypeB. Generally, most OnMessage handlers will ignore all but a handful of specific message types.
Of course, maybe you DON''T want MTypeB to be handled both as A and B, and still want the if/else if chain to differentiate between them and handle them completely differently. In that case, you then always have to be mindful of the message class hierarchy(in my opinion this is a bad plan... we shouldn''t have to know the relationship between MTypeA and MTypeB in order to handle them properly), or we simply required that all derived messages must use MGeneric as their immediate parent(not ideal either, especially when you come up with a message type that would be most easily created by extending another message class).
class MTypeA: public MGeneric(){};
class MTypeB: public MTypeA(){};
bool CDerivedMessageHandler::OnMessage(MGeneric& message)
{
if(dynamic_cast<MTypeA*>(&message))
{
/*respond to type A message*/
}
else if(dynamic_cast<MTypeB*>(&message))
{
/*respond to type B message*/
}
else
{
/*some other message*/
}
}
If an instance of CDerivedMessageHandler gets an MTypeB message, then as it is currently written, it will handle the message as though it were an MTypeA, because MTypeA is a base class of MTypeB.
But that''s not necessarily a bad thing. Maybe we rewrite the OnMessage handler like so:
bool CDerivedMessageHandler::OnMessage(MGeneric& message)
{
bool bResult=false;
if(dynamic_cast<MTypeA*>(&message))
{
/*respond to type A message*/
bResult=true;
}
if(dynamic_cast<MTypeB*>(&message))
{
/*respond to type B message*/
bResult=true;
}
return(bResult);
}
For a relatively small number of messages, this isn''t too bad. In this case a MTypeB would be handled both as a MTypeA AND as an MTypeB. Generally, most OnMessage handlers will ignore all but a handful of specific message types.
Of course, maybe you DON''T want MTypeB to be handled both as A and B, and still want the if/else if chain to differentiate between them and handle them completely differently. In that case, you then always have to be mindful of the message class hierarchy(in my opinion this is a bad plan... we shouldn''t have to know the relationship between MTypeA and MTypeB in order to handle them properly), or we simply required that all derived messages must use MGeneric as their immediate parent(not ideal either, especially when you come up with a message type that would be most easily created by extending another message class).
You are right about the problem that arises when you handle a base class prior to a derived one. It´s a pity compilers (vc7.1 at least) don´t warn about this. But it ocurred to me that there is a very similar situation where the compiler helps, exception handling. So I tried the twisted:
struct A : MGeneric {
};
struct B : A {
};
struct CDerivedMessageHandler {
bool OnMessage(MGeneric& message);
};
bool CDerivedMessageHandler::OnMessage(MGeneric& message)
{
try {
throw &message
} catch(A*) {
std::cout << "A\n";
} catch(B*) {
std::cout << "B\n";
} catch(MGeneric*) {
std::cout << "...\n";
}
}
But it doesn´t work. It appears the type of an exception is the static type of the variable that holded it, so the above code always catches MGeneric*. The Java version works as we want, though. However, IIRC, the java compiler wasn´t always smart enough to tell you that you had already catched some type of exception all the time.
You can escape derived base problem with:
#include "stdafx.h"
#include <iostream>
struct Base { int x; };
struct Derived : Base { int y; };
struct MGeneric {
virtual ~MGeneric() {}; // so that dynamic_cast works
};
template
struct Holder : T, MGeneric {
};
void OnMessage(MGeneric& message)
{
if( Base* b = dynamic_cast< Holder<base>* >(&message) )
std::cout << "B\n";
else if( Derived* d = dynamic_cast< Holder* >(&message) )
std::cout << "D\n";
}
int main() {
OnMessage(Holder());
}
Note that the solution above is almost boost::any. You can also use typeid instead of dynamic_cast to avoid subclassing problems.
Bruno
[edited by - br1 on December 31, 2003 5:53:31 PM]
struct A : MGeneric {
};
struct B : A {
};
struct CDerivedMessageHandler {
bool OnMessage(MGeneric& message);
};
bool CDerivedMessageHandler::OnMessage(MGeneric& message)
{
try {
throw &message
} catch(A*) {
std::cout << "A\n";
} catch(B*) {
std::cout << "B\n";
} catch(MGeneric*) {
std::cout << "...\n";
}
}
But it doesn´t work. It appears the type of an exception is the static type of the variable that holded it, so the above code always catches MGeneric*. The Java version works as we want, though. However, IIRC, the java compiler wasn´t always smart enough to tell you that you had already catched some type of exception all the time.
You can escape derived base problem with:
#include "stdafx.h"
#include <iostream>
struct Base { int x; };
struct Derived : Base { int y; };
struct MGeneric {
virtual ~MGeneric() {}; // so that dynamic_cast works
};
template
struct Holder : T, MGeneric {
};
void OnMessage(MGeneric& message)
{
if( Base* b = dynamic_cast< Holder<base>* >(&message) )
std::cout << "B\n";
else if( Derived* d = dynamic_cast< Holder
std::cout << "D\n";
}
int main() {
OnMessage(Holder
}
Note that the solution above is almost boost::any. You can also use typeid instead of dynamic_cast to avoid subclassing problems.
Bruno
[edited by - br1 on December 31, 2003 5:53:31 PM]
I''ve been thinking about this stuff the last week (and as usual, feedback from others has helped refine what I want to do). I have come up with the following new methodology to come up with to solve this particular problem, and i like the new methodology better.
First, I still want to have a messageid_t, held within MGeneric as the primary way of identifying messages. However, I will be abandoning the use of an integer type, and switch instead to a function pointer, like so:
For a moment, I attempted to come up with a way to make it so that if A were a messageid_t, then both x=A; and x=A(); would give me the same value. In order for that to work, however, it would require an infinitely recursive return type for A, or it would have to be a void*, which would be even worse, so I decided on a void function pointer with no parameters, so if I do accidentally put the () after it, the compiler will yell at me.
With this, the need for FMessageID is gone, and good riddance.
Defining values for messageid_t becomes much simpler, like so:
This is much better. A single line rather than three lines scattered over two files.
For additional information about a message, I have decided on using decorator classes and multiple inheritance, like so:
The decorators could be templates, like the holder one you illustrated.
First, I still want to have a messageid_t, held within MGeneric as the primary way of identifying messages. However, I will be abandoning the use of an integer type, and switch instead to a function pointer, like so:
typedef void (*messageid_t)();
For a moment, I attempted to come up with a way to make it so that if A were a messageid_t, then both x=A; and x=A(); would give me the same value. In order for that to work, however, it would require an infinitely recursive return type for A, or it would have to be a void*, which would be even worse, so I decided on a void function pointer with no parameters, so if I do accidentally put the () after it, the compiler will yell at me.
With this, the need for FMessageID is gone, and good riddance.
Defining values for messageid_t becomes much simpler, like so:
class CSomething
{
public:
static void MSGID_DoSomething(){}
}
This is much better. A single line rather than three lines scattered over two files.
For additional information about a message, I have decided on using decorator classes and multiple inheritance, like so:
class CXClient
{
private:
int m_nX;
protected:
void SetX(int nX){m_nX=nX;}
public:
CXClient(int nX):m_nX(nX){}
virtual ~CXClient(){}
int GetX()const{return(m_nX);}
};
class MXClient: public MGeneric, public CXClient
{
public:
MXClient(messageid_t messageid,int nX):MGeneric(messageid),CXClient(nX){}
};
The decorators could be templates, like the holder one you illustrated.
I asked because I thought that you may have solved the problem of not being able to use a switch.
The static object is also a one-liner:
struct s {
static const int i = 0;
};
I still wonder what was the problem of using RTTI. I currently favor the use of typeid. The comparison of type_infos should be optimizable to the comparison of vtable pointers. Also, if you are dynamic_casting for safety after you compared messageid_ts, the cost shouldn''t be a concern to you. What am I missing?
Bruno
The static object is also a one-liner:
struct s {
static const int i = 0;
};
I still wonder what was the problem of using RTTI. I currently favor the use of typeid. The comparison of type_infos should be optimizable to the comparison of vtable pointers. Also, if you are dynamic_casting for safety after you compared messageid_ts, the cost shouldn''t be a concern to you. What am I missing?
Bruno
the use of messageid_t is to indicate what sort of message has been sent, which in theory should indicate what sorts of extra data is contained within. determining the messageid_t value does not require a dynamic_cast, and does not require that we know anything about the MGeneric object hierarchy, nor does it require we make use of decorators(while I see using decorators as a good idea, I don''t want to require their use).
consider an application with many types of messages, with an OnMessage handler like the following:
For each message type, there is a dynamic cast. If we sent this handler a MTypeZ (one not handled by the function), it would go through all 25 casts, only to determine that it was "none of the above", and just return false.
With a messageid_t, there is no dynamic_cast to determine the type of message, just a comparison with a function pointer, which undoubtedly comes from an inline function. Only after an identifier is found is the dynamic_cast performed, as needed, giving you at most a single dynamic_cast.
consider an application with many types of messages, with an OnMessage handler like the following:
bool CDerivedMessageHandler::OnMessage(MGeneric& Message)
{
if(MTypeA* pA=dynamic_cast<MTypeA*>(&Message))
{
//type A
return(OnTypeA(*pA));
}
else
if(MTypeB* pB=dynamic_cast<MTypeB*>(&Message))
{
//type B
return(OnTypeB(*pB));
}
else
.
.
.
if(MTypeY* pY=dynamic_cast<MTypeY*>(&Message))
{
//type Y
return(OnTypeY(*pY));
}
else
{
//none of the above
return(false);
}
}
For each message type, there is a dynamic cast. If we sent this handler a MTypeZ (one not handled by the function), it would go through all 25 casts, only to determine that it was "none of the above", and just return false.
With a messageid_t, there is no dynamic_cast to determine the type of message, just a comparison with a function pointer, which undoubtedly comes from an inline function. Only after an identifier is found is the dynamic_cast performed, as needed, giving you at most a single dynamic_cast.
I share your concerns about dynamic_cast''s efficiency. However, with typeid it''s another story. You only have to ask the object for it''s type once. AFAIK, the compiler can optimize type_info comparison to simple pointer comparison, so the code in assembler is exactly yours, but without the trickery.
Have you looked at boost::any? At this point I think it is what I would use. I''s a single header and has a lot of extras we don''t need yet, but may in the future, like copy semantics. It''s implementation uses type_info so it''s efficiency shouldn''t be a concern.
Bruno
Have you looked at boost::any? At this point I think it is what I would use. I''s a single header and has a lot of extras we don''t need yet, but may in the future, like copy semantics. It''s implementation uses type_info so it''s efficiency shouldn''t be a concern.
Bruno
over the weekend, i gave using function pointers as messageid_t, errorid_t and stateid_t values a whirl (which replaced the old FMessageID and FErrorID classes i had in place for assigning unique values). it was a phenominal success, even with stateid_t (which i use for state machines). in the case of stateid_t, it replaced enumerated values (which i never did like using) that had a "reserved" identifier called STATEID_Null, which was -1, and the stateid_t values were used as an index to a vector of IState pointers. the vector has been replaced with a map<stateid_t,IState*>, and this removes the need for me to assign stateid_t values in a single place to guarantee uniqueness.
Topic Locked
This topic has been locked by a moderator. New replies are not allowed.
Sign in to reply to this topic.
Advertisement