   Pointer serialization (please optimize)
  I feel that many serialization examples are pretty hard to follow, but I've been trying to do something similar myself. It seems like this is a possible way to do it, but it may not be the best.Could you please give me some pointers on how to improve it? I've made the classes as simple as possible, but I want to use it on a dialogue system.You can find it here: Dialogue SystemI've made a simple test program for it as well.#include <iostream>#include <string>#include <fstream>using namespace std;class cClass {string Name;public:void SetName(string n) {Name = n;}string GiveName() {return Name;}};class cChar {string Name;cClass* Class;public:void SetName(string n) {Name = n;}string GiveName() {return Name;}void SetClass(cClass* c) {if(c != NULL) Class = c;}cClass* GiveClass() {if (Class != NULL) return Class;}int Serialize(); // 0 fail, 1 successint DeSerialize(cClass* Class);};int cChar::Serialize(){ofstream out;out.open("test.txt");if(!out) {cout << "Cannot open file.\n";return 0;}out << Name << "\n";out << GiveClass()->GiveName();out.close();}int cChar::DeSerialize(cClass* Class){ifstream in;in.open("test.txt");if(!in) {cout << "Cannot open file.\n";return 0;}string Temp;in >> Name;in >> Temp;Class->SetName(Temp);SetClass(Class);}int main(){cChar Char;cClass Class;cClass* pClass = &Classstring input;while (input != "q"){cout << "(S)erialize/(D)eSerialize/(Q)uit?" << "\n";cin >> input;if (input == "s"){string Temp;cout << endl;cout << "Class : ";cin >> Temp;Class.SetName(Temp);cout << endl;cout << "Name : ";cin >> Temp;Char.SetName(Temp);cout << "Class : ";cin >> Temp;Char.SetClass(pClass);Char.Serialize();}else if(input == "d"){Char.DeSerialize(pClass);cout << endl;cout << Char.GiveName() << "\n";cout << Char.GiveClass()->GiveName() << "\n";}}return 0;}[edited by - Corvwyn on February 14, 2004 4:35:56 PM]
Don't hard-code the stream into the function. Let it be a function parameter:class cClass{  std::string Name_;  friend std::ostream& operator<<(std::ostream& os, const cClass& cc);  friend std::istream& operator>>(std::ostream& is, cClass& cc);};std::ostream& operator<<(std::ostream& os, const cClass& cc){  return os << cc.Name_;}std::istream& operator>>(std::ostream& is, cClass& cc){  return is >> cc.Name_;}  Prefer references to pointersEither throw an exception on serialisation failure, or rely on the stream's fail() member function.When serialised objects 'contain' other serialised objects, give them all serialisation functions and have them call each other.If you're going to save the data using formatted IO, overloading << and >> does make more sense.Why is the cClass* in cChar a pointer? If it is a separate object, it should be serialized separately, storing a class_id into the cChar instead of using an actual pointer (and somewhere in your program, you keep a table class_id/class object). Ask if you want more details.Again, personal taste, 'Give' and 'Set' are redundant - if you pass a parameter to an accessor, then you're obvously 'setting', if you don't you're obviously 'getting'.  cClass& Name(std::string name)  {Name_ = n;return *this;  // Allows chained calls o.f(1).g(2).h(3);  }  // Query functions should be const so that they can be used on const objects  std::string Name() const  {return Name_;  }    The 'next' step in a serialisation system is to have the objects directly created by reading the file, but then I guess you'll say it's pretty hard to follow. Writing single characters is better done as characters rather than as strings: '\n' vs "\n". “Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it.”    — Brian W. Kernighan (C programming language co-inventor) [edited by - Fruny on February 14, 2004 5:23:27 PM];  Thanks for the reply. I understood most of it and I'm fixing up my code now. I knew I could overload >>/<< to get something done, but couldn't quite remember what for. Clever. I does take a bit more space though. I thought about having Serialize for both classes, I'm going to now.I've always been used to having pointers to (like cClass* in cChar). Using an id and getting the object from a list could also be done. If you think that would be best, please give me some pointers on how to do that (saves me some time finding out).  I'll make reply with my revised code later and see if I can take it one step at a time. If I just work on it I should be able to create objects directly from file (eventually).   [edited by - Corvwyn on February 14, 2004 10:47:26 PM];I''ve always been used to having pointers to (like cClass* in cChar). Using an id and getting the object from a list could also be done. If you think that would be best, please give me some pointers on how to do that (saves me some time finding out).If cClass is small, and each cChar gets one, make cClass directly a member, instead of having a pointer. If cClass is large, then make it a pointer, but have it be created/destroyed along with cChar. In both cases, serializing a cChar object means serializing a cClass object at the same time, only the creation/destruction semantics change.Now, what you seem to want is to share cClass objects between cChar instances. What this implies is that when serializing a cChar object, you don''t need to serialize a whole cClass object. Indeed, the two should be kept separate (cClass belong to ''game config'' data, while cChar belongs to ''saved game'' data).So what you''re doing is, when you start your program (or at any other suitable moment), you read in the file that contains the class information (which could be the same as the file with character information - it''s just that the two objects are not intermeshed), along with an identifier of some kind which lets you retrieve the class in a table (could be a number, could be a string...)Then, cChar objects which need to ''point'' to a cClass object will serialize, not the cClass object itself, not the pointer to the cClass object, but that identifier.Roughly speaking, the code would look something like so : struct cClass  // Not bothering with private/public{   typedef int id_type;   id_type Id_;   std::string Name_;};std::istream& operator>>(std::istream& is, cClass& object){   is >> object.Name_ >> object.Id_;   return is; }// I don''t really care about *saving* cClass objects, since// I assume they are hand-written configuration data about// what a class can and cannot do, stuff like that.struct cChar{   std::string Name_;   cClass* Class_;}std::ostream& operator<<(std::ostream& os, const cChar& object){   os << object.Name_ << '' '';   os << object.Class_.Id_ << '' '';   // note the extra space at the end, to delimitate in between objects   return os;}std::istream& operator>>(std::istream& is, cChar& object){   is >> object.Name_;   cClass::id_type id;   is >> id;   object.Class_ = class_table[id];   return is; }What is missing is, of course, theclass lookup code. class ClassTable{public:   typedef std::map<cClass::id_type, cClass*> table_type;private:   table_type table_;public:   ClassTable() {}   ~ClassTable()   { table_type::iterator itor; for( itor = table_.begin(); itor != table_.end(); ++itor )    delete itor->second;    // itor->first is the key (id)    // itor->second is the value (pointer)   }   ClassTable& AddEntry(cClass* cls)   { std::pair<cClass::id_type, cClass*> entry(cls->id, cls); std::pair<table_type::iterator, bool> result; result = table_.insert(entry); if (result_.second == false) {    // deal with failure } return *this;   }   cClass* operator[](const cClass::id_type& id) const   { table_type::iterator itor; itor = table_.find(id); if(itor == table_.end())    return 0; // not found, return null pointer else    return itor->second;   }};or similar.So, first, you would go around loading cClass objects, stuffing them into your class table, then you would use the class table to do the lookups. You could even do completely away with pointers and only keep the IDs in the objects, doing a lookup everytime it is needed. Though I would suggest you *at least* keep a pointer to the object while you''re manipulating it, instead of doing multiple lookups withing the same functions. Performance issues you know (std::map lookup is O(log n)).   “Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it.”    — Brian W. Kernighan (C programming language co-inventor);  Off topic:What is this?cClass* GiveClass() {if (Class != NULL) return Class;}If your compiler compiles that without complaining, kick it. Then replace it with a better one.   [ MSVC Fixes | STL Docs | SDL | Game AI | Sockets | C++ Faq Lite | Boost  Asking Questions | Organising code files | My stuff | Tiny XML | STLPort] ;  That, Kylotan would be a function for returning the class since accessing the object directly could be a problem at times (private). The whole if(Class != NULL) thing shouldn't be there though.I'd rather hope compilers handles that or I'd be pissed.   [edited by - Corvwyn on February 15, 2004 12:28:23 PM];  Brilliant fruny. I think I've gotten the idea now.I haven't overloaded any operators here. I know how to do it, but I like this way better at the moment. I'm going to do some experiments with file I/O now, since I haven't done much of that. #include <iostream>#include <string>#include <vector>#include <map>#include <fstream>using namespace std;class cClass {int id;string Name;public:cClass() {}int GiveId() {return id;}void SetId(int i) {id = i;}void SetName(string n) {Name = n;}string GiveName() {return Name;}int DeSerialize();};int cClass::DeSerialize(){ifstream in("class.txt");if(!in) {cout << "Cannot open file.\n";return 0;}in >> Name;in >> id;in.close();return 1;}class ClassTable {typedef map<int, cClass*> table_type;table_type table;public:ClassTable& AddEntry(cClass* cls){pair<int, cClass*> entry(cls->GiveId(), cls);pair<table_type::iterator, bool> result;result = table.insert(entry);if (result.second == false){cout << "Error" << endl;}return *this;}cClass* GiveClass(int id){ table_type::iterator itor; itor = table.find(id); if(itor == table.end())    return 0; // not found, return null pointer else    return itor->second;}};class cChar {string Name;cClass* Class;public:void SetClass(cClass* c) {if(c != NULL) Class = c;}cClass* GiveClass() {return Class;}void SetName(string n) {Name = n;}string GiveName() {return Name;}int Serialize(); // 0 fail, 1 successint DeSerialize(ClassTable* table);};int cChar::Serialize(){ofstream out("char.txt", ios::app);if(!out) {cout << "Cannot open file.\n";return 0;}out << Name << endl;out << GiveClass()->GiveId() << endl;out.close();return 1;}int cChar::DeSerialize(ClassTable* table){ifstream in("char.txt");if(!in) {cout << "Cannot open file(char).\n";return 0;}int id;in >> Name;in >> id;SetClass(table->GiveClass(id));in.close();return 1;}int main(){cChar Char;cClass Class[3];cClass* pClass[3];for(int i=0;i<3;i++)pClass[i] = &Class[i];ClassTable Table;ClassTable* pTable = &TableClass[0].DeSerialize();Table.AddEntry(pClass[0]);string input;while (input != "q"){cout << "(S)erialize/(D)eSerialize/(Q)uit?" << endl;cin >> input;if (input == "s"){string Temp;int id;cout << endl;cout << "Name : ";cin >> Temp;Char.SetName(Temp);cout << "Class : ";cin >> id;Char.SetClass(Table.GiveClass(id));Char.Serialize();Char.SetName("");Char.SetClass(NULL);}else if(input == "d"){Char.DeSerialize(pTable);cout << endl;cout << Char.GiveName() << endl;cout << Char.GiveClass()->GiveName() << endl;}}return 0;} [edited by - Corvwyn on February 19, 2004 3:47:03 AM];quote:Original post by Corvwyn  That, Kylotan would be a function for returning the class since accessing the object directly could be a problem at times (private). The whole if(Class != NULL) thing shouldn''t be there though.I''d rather hope compilers handles that or I''d be pissed.   [edited by - Corvwyn on February 15, 2004 12:28:23 PM]I''d rather hope it wouldn''t. What is it supposed to return if Class == NULL? [ MSVC Fixes | STL Docs | SDL | Game AI | Sockets | C++ Faq Lite | Boost  Asking Questions | Organising code files | My stuff | Tiny XML | STLPort] ;  No, you're right. I should get an "all paths must return a value" error. I'm using .NET 2003 by the way. (yeah, yeah... I like it)Anyone care to comment on my serialization by the way? (Yeah, I nag...)   [edited by - Corvwyn on February 18, 2004 2:47:25 PM];  Write error messages to stderr, not to stdout.I do hope you''ve done some research and learned how std::map worked, not just cut & pasted my code.Why are you allocating an array of cClass AND an array of pointers? Indeed, why not just let them live inside the map, and let it clean things up, as I did in my version? It becomes very useful once you start dynamically creating objects based on the data in the file. “Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it.”    — Brian W. Kernighan (C programming language co-inventor)