#include <map>
#include <memory>
#include <exception>
#include <vector>
#include <iostream>
template <class Base, class Derived>
static Base* create()
{
std::cout << "." << std::endl;
return new Derived;
}
template <class Base, typename Key = int>
class SimpleFactory
{
// For simpler handling of the function pointer
typedef Base* (*create_func)();
// This map stores a creator function for a type
typedef std::map<Key, create_func> creator_map;
creator_map c_map_;
//static creator_map c_map_;
public:
template <class Derived>
void Register(Key const& key)
{
if (!c_map_.insert(std::make_pair<Key, create_func> (key, &create< Base, Derived> ) ).second)
throw 0; //throw something meaningful later
}
Base* GetInstance(Key const& key) const
{
typename creator_map::const_iterator iter = c_map_.find(key);
if (iter != c_map_.end())
return ( (iter->second)() ) ;
else
throw 0;
}
};
class B {
public:
virtual void Serialize() = 0; //more common to all Derived classes
//virtual void DeSerialize() = 0; //more common to all Derived classes
//more common to all Derived classes
virtual ~B() {}
};
class D1 :public B {
int index;
public:
D1() : index ( 0 ) {}
virtual ~D1() {}
void Serialize() {
index ++;
std::cout << "[Working=" << std::hex << index << " ]" << std::endl;
}
int GetIndex() {
return ( index ) ;
}
void Increment() { std::cout << "Doing it" << std::endl; }
//more functions specific to D1
};
class D2 :public B {
public:
virtual ~D2() {}
void Serialize() { std::cout << "Working" << std::endl; }
void DoAnotherThing() { std::cout << "Doing it" << std::endl; }
void Try() { std::cout << "Trying 2" << std::endl; }
};
int main() {
static SimpleFactory<B, int> simple;
simple.Register<D1>(1);
D1* ptr = static_cast< D1* > ( simple.GetInstance ( 1 ) );
ptr->Serialize();
D1* ptr2 = static_cast< D1* > ( simple.GetInstance ( 1 ) );
std::cout << "[] " << ptr2->GetIndex() << std::endl;
std::cin.get();
}
How do I get _any/all_ changes to D1 reflected in all instances (i.e ptr or ptr2) of D1 without making D1 a singleton? In other words. The call ptr2->GetIndex() should display 1 as opposed to zero since the member variable 'index' was incremented in the Serialize method.
I'm not sure how you could do it without making it a singleton. The index attribute is non-static as is the serialize function. Therefore each object has its own copies of the class attributes. I'm not sure why you expected the serialize function to do anything special for you.
I'm not sure how you could do it without making it a singleton. The index attribute is non-static as is the serialize function. Therefore each object has its own copies of the class attributes. I'm not sure why you expected the serialize function to do anything special for you.
OK! Let me rethink this.
Here's another question: Assume the messages were singleton.
template <class Base, class Derived>
static Base* create()
{
std::cout << "." << std::endl;
return new Derived;
}
Since the Derived type was is wrapped in a Singleton, I'm assuming there's nothing inherently wrong calls to "Create" - after initial call (ptr). True?
template <class Base, class Derived>
static Base* create()
{
std::cout << "." << std::endl;
return new Derived;
}
Since the Derived type was is wrapped in a Singleton, I'm assuming there's nothing inherently wrong calls to "Create" - after initial call (ptr). True?
I just think that your approach is way too convuluted. Why do you need a serialize function? If the factory is supposed to return the same object provided that you request using the same key then what is the problem? Actually I have to rethink my first post because the class in question doesn't need to follow the singleton pattern. If all users are asking the factory for the instance by the key then the factory simply needs to check whether the instance was already created and return its pointer. I'm not sure what you are doing with the function pointer and the templates. Sorry I don't have time to debug it myself but have you used a debugger to confirm that only 1 object of type D1 gets created? I suggest that you add a std::cout into the constructor body and confirm that there is only one object being created.
If you want this to work you have to ensure that the factory is creating only one object for each key value.
I just think that your approach is way too convuluted. Why do you need a serialize function?
For starters, the pure virtual 'Serialize' function in the Base class is contrived. An alternate name might have eliminated the confusion. That said,...... consider this. Lets assume you and I are having a conversation. When you communicate with me you send me a Dn message, where n = 1,2,3..N. When I communicate with you, I send you a Pn message. Dn and Pn messages are derived off Base (B). Read on ...
Originally Posted by kempofighter
Actually I have to rethink my first post because the class in question doesn't need to follow the singleton pattern. If all users are asking the factory for the instance by the key then the factory simply needs to check whether the instance was already created and return its pointer
That's what I'm after. From my perspective, there's a handful of your (Dn) messages that upon receipt (Read) I need to examine a particular Pn. IOW I need to look at what I last told you. So now pseudo code.
Code:
typedef std::vector < unsigned char* > BUFFER ;
BUFFER buffer ( MAX_SIZE ) ;
void SomeClass::Read () {
//the Read function handles all your incoming message. A raw buffer is turned into approprate
// Dn Message type
unsinged int const numBytesReceived = obj.Read ( &buffer [ 0 ], DESIRED_SIZE ) ;
if ( numBytesReceived <= 0 ) return ;
eventHandler.Notify ( buffer ) ;
}
//we're here now
void ProcessD1Handler::Process ()
{
// 1) Here I'll turn the RAW char* buffer into - lets say a D1 message (see yourD1 obj)
// 2) Later
if ( myP1.GetTargetValue() <= MAX_POS ) {
//DO Nothing with your data... The scan position hasn't changed.
} else {
unsinged int const YourValue = yourD1.GetSomeValueFromYou() ;
//lots of other stuff
}
}
NOTE: There's another side of this I'm not showing. i.e I'm also communicate with 4 others (you across 1 interface and 4 others across another interface).
Long story short, I decided to use a map to 'hold your message'. IOW I need a 'global' registry of sorts to house the Pn messages - such that within _any_ ProcessDnHandler I could look at a particular Pn. (hence the factory and the singleton messages. I'm open to ideas if you see a better solution).
Originally Posted by kempofighter
Sorry I don't have time to debug it myself but have you used a debugger to confirm that only 1 object of type D1 gets created? I suggest that you add a std::cout into the constructor body and confirm that there is only one object being created.
Sure... I'll get around to that when I get back to my machine which has a compiler
With all the respect, I recommend that you follow kempofighter's suggestion.
IMHO, the design shouldn't be any more complicated than returning a valid pointer to a valid instance.
Your current design also needs referencing counting otherwise it is likely to result in undefined state
With all the respect, I recommend that you follow kempofighter's suggestion.
QUOTE]
I did and came up with a revised implementation. I still have a dilema, nonetheless you should be able to execute the attached source. So now:
Code:
#include <iostream>
#include <typeinfo>
#include <vector>
/// BEGIN "UNTOUCHABLE" CLASSES AND DATA INSTANTIATORS ///////////////
class base {
public :
//handful of stuff
virtual ~base() {}
};
/*
one of many untouchable pn derived classes with
methods not found in base class
*/
class p1 : public base {
int value ;
public :
p1()
: value ( 0 )
{}
void set_something ( int in ) { value = in ; }
int get_something() const { return ( value ) ; }
~p1() {}
};
class p2 : public base {
public :
//stuff
~p2() {}
};
//up to pn
/// END "UNTOUCHABLE" Pn CLASSES AND DATA INSTANTIATORS /////////////////
/*
============================================================
holder is a collection of UNTOUCHABLE classes, with useful
member data (For example) 'count'. For each call to Set_ a phandler will
increment count
*/
typedef unsigned int unsigned_type ;
struct holder {
unsigned_type holder_id ;
unsigned_type count ;
static base *ptr ;
holder ( base *ptr_, unsigned_type const id )
: holder_id ( id )
{
ptr = ptr_;
}
virtual ~holder() { }
};
base* holder::ptr = 0 ;
typedef std::vector < holder* > holder_vec ;
/*
============================================================
base handler for d's.
notified upon receipt of a client message
*/
class d_handler_base {
public :
virtual void process() = 0 ;
virtual ~d_handler_base() {}
};
class d1_handler : public d_handler_base
{
holder_vec hv ;
//only interested in p1 objects located in the holder containers
p1* ptr_p1 ;
public :
d1_handler ( holder_vec& in )
: hv ( in )
//NOTE!!!!!!!!!!!!!!!!!
//ideally i need to use a map so i could interate and FIND p1
//for now we'll assume p1 exists at location 0 within holder_vec
, ptr_p1 ( static_cast < p1* > ( hv [ 0 ]->ptr ) )
{}
void process ()
{
unsigned_type const dummy = ptr_p1->get_something() ;
std::cout << std::hex << "0x" << dummy << std::endl;
}
};
class d2_handler : public d_handler_base
{
public :
d2_handler ()
{}
void process ()
{
std::cout <<"." << std::endl;
}
};
/*
============================================================
base handler for p's.
notified upon receipt of a server message
*/
class p_handler_base {
public :
virtual ~p_handler_base() {}
};
class p1_handler : public p_handler_base
{
holder_vec& hv;
p1* ptr_p1 ;
public :
p1_handler ( holder_vec& in )
: hv ( in )
, ptr_p1 ( static_cast < p1* > ( hv [ 0 ]->ptr ) )
{}
void update_p_msg ( unsigned_type const in ) {
ptr_p1->set_something ( in ) ;
hv [ 0 ]->count ++ ;
}
};
int main() {
static holder_vec hv ;
//this 'new new' stuff seems unsightly... i think a map would be better....
// for now create a p1 with its id of 0x400, similarily a p2 with it's id 0x500
hv.push_back ( new holder ( new p1(), 0x400 ) ) ;
hv.push_back ( new holder ( new p2(), 0x500 ) ) ;
d1_handler *ptrd1 = new d1_handler ( hv ) ;
p1_handler *ptrp1 = new p1_handler ( hv ) ;
//change
ptrp1->update_p_msg( 0xCAFE ) ;
ptrd1->process() ;
//change
ptrp1->update_p_msg( 0xDEAD ) ;
ptrd1->process() ;
std::cin.get() ;
}
In a nutshell:
Common message holder class (named holder) with a collection of base classes. Within main a p1_handler object initiates a change to a p1 object (via the call to set_something). The d1_handler observes the change.
At this juncture you may say this ‘screams’ an event handler (perhaps observer implementation). This way any change to a Pn message by a Pn handler is dispatched to a Dn handler. My answer to an event handler (observer implementation) is No! Here’s why: Referencing the d1 and d2 handler classes
a) Not all Dn handlers is interested in Pn (UNTOUCHABLE) messages. In this case the message holder vec with Pn message is a _dont_care_ for a d2_handler.
b) For those Dn handler interested in Pn messages (i.e d1_handler), these Dn handlers are not interested in being notified of immediate changes to Pn messages by Pn handlers. These Dn handlers is only interested in knowing they have the ‘latest’ Pn message _whenever_ (600 updates could have occurred since) they opt to access a Pn message object.
Alas here’s where I still struggle with C++. The design and what (perhaps) pattern to use to objective. After scanning the GOF here are my thoughts:
1) Leave well enough alone. Current implementation works but then I’m often not satisfied given my struggles
2) A proxy class between the Pn handlers and Dn handlers. I suspect this would be beneficial to the Dn handlers (more so that Pn handler since a Pn message could be local to a Pn handler). In doing so the Dn handler could request via the Proxy for a Pn message and receive the ‘lastest/most up to date’ Pn message. The drawback here is the overhead and (worse) I’m not sure how to implement a proxy (at least not yet)
3) Perhaps a common base class between Pn handlers and Dn handlers would suffice.
1. Draw a visual schema for the current design. You've got Pb, P1,.. Pn, Db, D1... Dn, with a staic pointer to Pb, and Db(n) handlers contain pointers to Pb. You seriously need to implement some sort of referencing counting.
2. Focus the design of the handlers according to WHAT they do, not WHEN they need to do.
3. A change is an event. Acting upon the 'latest message' is an event. Sockets and dispatch pattern feed upon this type of events. If you still think event handler doesn't go well with your current design, then who is responsible triggering a 'change'? the user?
4. Look at the code below
Code:
hv.push_back ( new holder ( new p1(), 0x400 ) ) ;
hv.push_back ( new holder ( new p2(), 0x500 ) ) ;
d1_handler *ptrd1 = new d1_handler ( hv ) ;
p1_handler *ptrp1 = new p1_handler ( hv ) ;
I guarantee that whoever uses your code must be a contributor to your design. A normal user like me would not dare to use it. In other words, design from the interface point of view, not the implementation.
IMHO, working around a simple packet might help to resolve the dilema you're facing.
something along the line of
Code:
#include <iostream>
#include <string>
#include <cstring>
#include <map>
// example flags
//////////////////////////////////////////////////////////////////////////////80
#define PK_REQ_NONE 0x00000000
#define PK_REQ_TYPE 0x00000001
#define PK_REQ_AUTH 0x00000002
#define PK_REQ_TIME 0x00000004
#define PK_REQ_ID 0x00000008
#define PK_REQ_SERVICE 0x0000000F
#define PK_REQ_S_POST 0x00000010
#define PK_REQ_S_GET 0x00000020
#define PK_REQ_S_NOTIFY 0x00000030
#define PK_REQ_S_DISCARD 0x00000040
#define PK_REQ_EXTRA 0x0000FFFF
#define PK_REQ_ERROR 0xDEADBEEF
struct HPacket
{
unsigned short hType;
unsigned short hAuth;
unsigned long hService;
unsigned long hTime; // timestamp
unsigned long hID; // packet/transmisiion id
unsigned long hExtra;
long hError;
};
struct Packet
{
HPacket pHeader;
char pData[1000];
};
typedef unsigned long DWORD_L;
class DataHandler
{
// modify accordingly. e.g., implement a new key or something
typedef std::map<DWORD_L, std::string> MapType;
public:
void PostMsg(const DWORD_L id, const std::string& msg)
{
msgHolder.insert(msgHolder.find(id), MapType::value_type(id, msg));
}
void CallBack(const DWORD_L id) const
{
// call appropriate handlers, e.g., p_handler_base
// or handlers derived from *this
}
// and whatever operations you need to perform
// ...
virtual ~DataHandler(){}
protected:
// decide the location, this is just an example
MapType msgHolder;
};
class PacketListener
{
public:
// modify accordingly also here
DWORD_L Listen(const Packet& pkt)
{
if(pkt.pHeader.hAuth != 666)
return PK_REQ_ERROR;
DWORD_L f = pkt.pHeader.hService;
if(f == PK_REQ_S_POST)
{
Post(pkt.pHeader.hID, pkt.pData);
}
else if(f >= PK_REQ_S_NOTIFY)
{
Post(pkt.pHeader.hID, pkt.pData);
Notify(pkt.pHeader.hID);
}
else
return PK_REQ_AUTH;
return PK_REQ_ERROR;
}
private:
void Post(const DWORD_L id, const std::string& msg)
{
std::cout << "Posting [" << id << "] " << msg << "\n";
dHandle.PostMsg(id, msg);
}
void Notify(const DWORD_L id) const
{
std::cout << "notifying the client, with ID# " << id << "\n";
dHandle.CallBack(id);
}
DataHandler dHandle;
};
namespace UTest
{
void Send(Packet& pkt, const std::string& msg)
{
strcpy(pkt.pData, msg.c_str());
}
void Recv(Packet& p)
{
PacketListener pd;
pd.Listen(p);
}
// scoped
using namespace std;
}
int main()
{
using namespace UTest;
Packet pkt;
pkt.pHeader.hTime = 1000;
pkt.pHeader.hID = 777;
pkt.pHeader.hAuth = 666;
// don't care about notifying, just post.
pkt.pHeader.hService = PK_REQ_S_POST;
Send(pkt, "Cow says hi!");
Recv(pkt);
Packet pkt2 = pkt;
pkt2.pHeader.hID = 766;
pkt2.pHeader.hTime = 2000; // denotes latest time stamp
// notify this message to the appropriate handler
pkt2.pHeader.hService = PK_REQ_S_POST | PK_REQ_S_NOTIFY;
Send(pkt2, "How says hi back!");
Recv(pkt2);
}
3. A change is an event. Acting upon the 'latest message' is an event. Sockets and dispatch pattern feed upon this type of events. If you still think event handler doesn't go well with your current design, then who is responsible triggering a 'change'? the user?
Again - from my perspective a Dn handler does not need a notification when a Pn message is updated by a Pn handler. A Dn handler's process method is invoked after 'Reads' of Aircraft (A/C) data. Only then will a Dn handler (for those interested in Pn messages) examine a Pn message - which should be the latest message. Pn messages are transmitted (Write) to the A/C.
Again the Pn message could be updated by a Pn handler a 1000 times and written to the A/C before a call to a Dn handler process methods. Until the Dn process method is called the Dn handler is simply not interested in Pn handler notifications of Pn messages.
Originally Posted by potatoCode
I guarantee that whoever uses your code must be a contributor to your design. A normal user like me would not dare to use it. In other words, design from the interface point of view, not the implementation.
IMHO, I'll make the same argument about your source. Source snipped..
Again - from my perspective a Dn handler does not need a notification when a Pn message is updated by a Pn handler.
Then, don't notify the Dn handler, it's simple as that.
A Dn handler's process method is invoked after 'Reads' of Aircraft (A/C) data. Only then will a Dn handler (for those interested in Pn messages) examine a Pn message - which should be the latest message. Pn messages are transmitted (Write) to the A/C.
So you're saying I should have clairvoyance and know what you're not showing? You're turning this whole post from "how do I get a valid instance pointer without turning handler a singleton?" to this? It doesn't matter if it reads Aircraft data or some porn data. It could read/write 1000000 terabytes of customers list of Walmart, it don't mean a thing here. The fact that Pn and Dn handlers work on the common data remains unchanged, and speaking of which are you properly synchronizing read/data? I doubt that you do
Again the Pn message could be updated by a Pn handler a 1000 times and written to the A/C before a call to a Dn handler process methods.
Again, it don't mean a thing how many times Pn handler does who knows what, Dn handlers at least at ONE point in event time line must query Pn handler, period.
Until the Dn process method is called the Dn handler is simply not interested in Pn handler notifications of Pn messages.
Then, don't call, what's the problem?
IMHO, I'll make the same argument about your source. Source snipped..
This was not about my code, but yours. However, I suppose you say that to whoever implements bit flags?
What else have you tried besides being bent on the notion that 'Dn handler is not simply interested'? If Dn handler is not interested in any/all/part, then why bother specializing separate handlers when Pn seems suffice? Have you thought about modeling your data structure first before coming up with an operational design? And frankly, what's the deal with this 'UNTOUCHABLE's? a mob? or is that a word better understood as an const object?
Stop being stubborn and explore options presented to you.
Are you required to not implement observers? It is mandatory to have two handler types?
Give a full heads up as to what the requirements are, not what you think is required.
Last edited by potatoCode; February 6th, 2010 at 10:35 PM.
It's your responsability to explain clearly what's your problem. Your code it's not clear and it's full of inconcistencies that make very hard to understand exactly what's your real intent.
As far as I could read from your code snippets you have two class hyerarchies P and D, a mechanism describing a set of fixed connections between pairs of P's and D's, a function (update_p_msg) issuing a message to some P and a function (process) notifing the message to the correspondingly connected D's. Am I right ?
As far as I could read from your code snippets you have two class hyerarchies P and D, a mechanism describing a set of fixed connections between pairs of P's and D's, a function (update_p_msg) issuing a message to some P and a function (process) notifing the message to the correspondingly connected D's. Am I right ?
Alas, I could dispatch the updated PnMsg from a PnHandler to the DnHandler every 40 ms, but why –
given DnHandler is not interested in a PnMsg until 200 ms later. I think that’s where the disagreements
between PotatoCode and I started. I feel a more suitable approach is to create a ‘global’ PnMsg
repository.
I don't see a big difference between the two options you describe. In one case you have a global repository in the other the repository is embedded in the class using it. So, IMO where you ask why, you should ask why not?
Cheers, D Drmmr
Please put [code][/code] tags around your code to preserve indentation and make it more readable.
As long as man ascribes to himself what is merely a posibility, he will not work for the attainment of it. - P. D. Ouspensky
* The Best Reasons to Target Windows 8
Learn some of the best reasons why you should seriously consider bringing your Android mobile development expertise to bear on the Windows 8 platform.