trouble saving/loading 'complex' classes

[Quibus]

Hey there,

I have been trying to get a working load and save function for loading and saving a class with a member vector class ... however.. due to the nature of 'vector' (without the vector <Item> items everything works like a charm) I'm unable to properly save my classes. Can anybody help me with this problem? I've been working on it for quite some time now and I just can't get it to work! Any examples/working code/suggestions are greatly appreciated!

I'm trying to load/save this class:

class Building
{
public:
    int x,y;
    int matrixx, matrixy;
    bool m,f;
    vector <Item> items ;

with Item being defined as:

class Item
{
public:
    string name$; 				
    string description$;		
    int kind; 					
    int nr;				        

To do this I am trying to use the following functions:

template<typename T>
void write(ofstream& out, T& t)
{
    out.write((char *) &t, sizeof(T));
}

template<typename T>
void read(ifstream& in, T& t)
{
    in.read((char *) &t, sizeof(T));
}

these functions are called by the following pieces of code:

    ofstream out(file_name.c_str(), ios::out);
    tempsize = 0 ;
    for(int q = 0; (unsigned int)q < buildings.size(); q++)
            tempsize ++ ; //*see comment below
    out << tempsize << " ";

    for(unsigned int q = buildings.size();  q --> 0 ; )
    {
        write(out, buildings[q]);
        buildings.erase (buildings.begin() + q);
    }
    out.close() ;

determining the size by using tempsize ++ instead of directly using .size() actually has a function in the real program, but I simplified this piece of code for this post.

        
    ifstream in(file_name.c_str(), ios::in);    

    in >> vectorsizebuildings;
    in.ignore(1); //ignoring the last " "
    if (vectorsizebuildings > 0)
        {
            Building tempbuilding ;
            for(int q = 0; q < vectorsizebuildings; q++)
            {
                read(in, tempbuilding);
                buildings.push_back(tempbuilding) ;
            }
        }
    in.close();

Is there anyone out there who can help me with this?

[Disch]

Your read/write functions are [potentially] very dangerous and I would not recommend ever doing that to read/write binary files.

I went over all the reasons for this somewhat recently, along with a proper "how to". The thread is here:

http://www.cplusplus.com/forum/beginner/76800/

If you're only interested in the how-to, you can find it here:

http://www.cplusplus.com/forum/beginner/76800/#msg412759

[Quibus]

You are correct of course, but the reason I'm still approaching it like this is mainly that I'm saving a large number of classes via these functions and that the classes are actually a lot bigger (30+ members). It simply saves a lot of work if I can use one standard function for it. I don't really mind that there are possible 'version incompatibility issues' since I'm always saving and loading the files on the same system and only while the program is running.. nothing is stored when the program is ended.

I don't see how I can efficiently make a template function using the methods you described without writing separate functions for each and every class (and class members!). I can make standard save/load functions per data type (int, double) and accessing them when going through each class but I just don't see a possibility to get around this (creating a class-specific save function for all my classes will take a huge ammount of work) to standardize the function for my classes. How would you handle this?

[Disch]

I don't see how I can efficiently make a template function using the methods you described without writing separate functions for each and every class (and class members!)

You can't. That suggested method requires each class to have its own read/write machanism.


Saving the class in a big clump will only work if it's a POD class, and if there is no dynamic memory allocation. Unfortunately neither of these are true when you're talking about vectors.... so I don't think there's any way to generically save/load a class that has a vector in it.

Perhaps you could minimize the damage by specializing the template for the class(es) that contain a vector, and save/load each of those members individually just for that class. That'll at least save you the trouble of doing it for every single class.

[Quibus]

You can't. That suggested method requires each class to have its own read/write machanism.

Yeah I was afraid of that..
I've now begun on working on getting it all to work using the method you described in that topic you linked to. However I do run into some trouble (I can get around it but I'm not sure if I can run into problems by ignoring these things) since you've shown such expertise on the topic I was hoping you could answer these questions:

So basically everything is functioning as I expected it but I'm really worried that I might run into unexplained problems in the future since I don't feel like I fully understand what I'm doing.

First of all.. implementing #include <cstdint> triggered

#error This file requires compiler and library support for the upcoming \
ISO C++ standard, C++0x. This support is currently experimental, and must be \
enabled with the -std=c++0x or -std=gnu++0x compiler options. 

in file "c++0x_warning.h". I just tried to ignore that error (simply used /* */on it) and it compiled just fine.. Should I be looking into something to make sure that I don't run into unexplained errors in the future?

I also ran into something in the other topic that was a little unclear to me. I only used standard types like int and double so far and I am currently trying to save and load them using

writeu32(fileout, nr);
nr = readu32(filein);

with nr representing a standard c++ Int

void writeu32(ofstream& fileout, u32 val) {
	char bytes[4];
	bytes[0] = (val & 0x000000FF);
	bytes[1] = (val & 0x0000FF00) >> 8;
	bytes[2] = (val & 0x00FF0000) >> 16;
	bytes[3] = (val & 0xFF000000) >> 24;
	fileout.write((char*) bytes, 4);
}

u32 readu32(ifstream& filein) {
	u32 val;
	char bytes[4];
	filein.read((char*) bytes, 4);
	val = bytes[0] | (bytes[1] << 8) | (bytes[2] << 16) | (bytes[3] << 24);
	return val;
}

I know that I am treating the 'nr' as an unsigned integer but I've checked it with negative numbers and it still works (I just tried it since I thought signed and unsigned 32bit integers both represented an equal ammount of storage space and that I could therefore simply use the same function for saving both :/ (not sure if this can run into problems in the future though)

I'm also not sure how to save standard doubles (can I just enter and handle them in a standard 8 bytes funtion?)

The posts you made in the other topic were really helpfull and without the replies you gave me in this topic I'd probably still be trying to get the original functions to work (which would complicate matters needlessly without learning anything). Thanks for that!

[Disch]

in file "c++0x_warning.h". I just tried to ignore that error (simply used /* */on it) and it compiled just fine..

A better thing to do would be to do what the error suggested, and compile with the -std=c++0x compiler option.

An even better thing to do would be to update your compiler, as C++11 has been the standard for over a year now and GCC (which I assume you're using), has had very respectable support for quite some time.

(I just tried it since I thought signed and unsigned 32bit integers both represented an equal ammount of storage space and that I could therefore simply use the same function for saving both :/ (not sure if this can run into problems in the future though)

That's fine. Really the only reason I separate signed/unsigned versions is for code clarity and to avoid having to cast back/forth between types. But if that doesn't bother you, then saving everything as unsigned will work just fine.

I'm also not sure how to save standard doubles (can I just enter and handle them in a standard 8 bytes funtion?)

floating points are tricky for a few reasons. One is that the format they're stored in does not seem to be as clearly regulated as integral types. Another is that you can't extract raw, individual bytes from them without doing really ugly memory tricks.

For these reasons I pretty much always write them as strings. It's not ideal because it's a little slower than it could be, but it's easy and reliable. Something like this should work for you:

void writedouble(ostream& fileout, double val)
{
  stringstream s;
  s << val;
  writestring(fileout,s.str());
}

double readdouble(istream& filein)
{
  double val = 0;
  stringstream s( readstring(filein) );
  s >> val;
  return val;
}

If you'd rather not do it that way... and if you're not worried about endian issues, then doing a dumb read/write would work just fine:

void writedouble(ostream& fileout,double val)
{
  fileout.write((char*)(&val), sizeof(val));
}

double readdouble(istream& filein)
{
  double val;
  filein.read((char*)(&val), sizeof(val));
  return val;
}

But again note that this would be subject to system endianness and floating point storage mechanisms which may vary from machine to machine. However if these files are not going to survive past the end of the program, then that's a nonissue.


Also side note: I used the more generic 'ostream' and 'istream' classes in my examples above because it's less restrictive. No point in restricting yourself to only fstreams if you're not doing anything that's specific to fstreams. But that's a minor detail -- I'm sure it won't make a difference for you.

[Quibus]

I wouldn't have known what to do without you. Thanks for all the help!

[Quibus]

one more problem :/....

I boiled it down to the following codesnippets

#include <cstdint>

typedef int32_t  s32;

void writes32(char bytes[4], s32 val) {
	bytes[0] = (val & 0x000000FF);
	bytes[1] = (val & 0x0000FF00) >> 8;
	bytes[2] = (val & 0x00FF0000) >> 16;
	bytes[3] = (val & 0xFF000000) >> 24;
}

s32 reads32(char bytes[4]) {
	s32 val;
	val = bytes[0] | (bytes[1] << 8) | (bytes[2] << 16) | (bytes[3] << 24);
	return val;
}



int main()
{

    char bytes[4] ;

    int in = 128 ;
    int out = 0 ;

    writes32(bytes, in);
    out = reads32(bytes);

    cout << "out:  " << out ;
    return 0;
}

the code works perfectly fine from in = 0(and below 0 for the most part) up until 'in' becomes 128. at that point I suddenly get values that are off (-128). when I look at the hex codes I can see that 0(hex 1) to 127(hex: 7f) works fine but when it gets to 128 (hex80) I get the outcome -128 (hex: ffffff80) Obviously I made a mistake somewhere, but what am I doing wrong in the code? (I thought I had it all working already but I initially only tested it for numbers that were lower than this)

[Disch]

use unsigned chars instead of signed chars. The sign bit is biting you.

// problem is here:
val = bytes[0] | (bytes[1] << 8) | (bytes[2] << 16) | (bytes[3] << 24);

say for example, bytes[0] is between 0x80 and 0xFF. This means it is treated as a negative number.

Now, all this math promotes the signed char to a signed int.

so if bytes[0] is 0xFF, then that means:

- bytes[0] = 0xFF
- bytes[0] = -1 (as a char)
- bytes[0] = -1 (promoted to an int)
- bytes[0] = 0xFFFFFFFF (promoted to an int)

The other option is to mask each byte before you OR them together:

val = (bytes[0] & 0xFF) | ((bytes[1] << 8) & 0xFF00) | ((bytes[2] << 16) & 0xFF0000) | ((bytes[3] << 24) & 0xFF000000);

[Quibus]

You make it all seem so simple :) Thanks again!