std::vector memory manipulation with serialization and deserialization

Question

This is my code and I would like to get it code reviewed. It is functional and behaves as expected.

I pass some basic types to the Serialize() function and then deserialize the output to get back the original value.

size_t returnSize(const char* s)
{
       string string(s);
       return string.size();
};
template<typename T>
size_t returnSize(const T& t)
{
       return sizeof(t);
};

template<typename T>
string Serialize(const T& t)
{
    T* pt = new T(t);
    vector<char> CasttoChar;

    for (int i =0 ;i<returnSize(t);i++)
    {
        CasttoChar.push_back(reinterpret_cast<const char*>(pt)[i]);
    }
    delete pt;
    return  string( CasttoChar.begin(), CasttoChar.end() );

};

template<>
string Serialize<string>(const string& t)
{
    return Serialize(t.c_str()); 
};

template<typename T>
T DeSerialize(const string& cstr)
{
    const T* a = reinterpret_cast<const T*>(cstr.c_str());
    return *a;
}

template<>
string DeSerialize<string>(const string& cstr)
{
    return DeSerialize<char*>(cstr);
}

int _tmain(int argc, _TCHAR* argv[])
{
    int x = 97;
    string c = Serialize<int>(x);
    cout << DeSerialize<int>(c) << endl;

    char g = 'g';
    string c2 = Serialize<char>(g);
    cout << DeSerialize<char>(c2) << endl;

    string k = "blabla";
    string c3 = Serialize<const char*>(k.c_str());
    cout << DeSerialize<char*>(c3) << endl;

    string ka = "string";
    string c4 = Serialize<string>(ka);
    cout << (DeSerialize<string>(c4)).c_str();

    system("PAUSE");
    return EXIT_SUCCESS;
}

Answer 1

First of all, fundamentally: std::string is a data type to hold text. Exclusively. It should not be used to hold binary data. Use a std::vector<(unsigned) char> for that.

Secondly, you are using heap allocation without needing to:

T* pt = new T(t);

This makes no sense at all, and introduces the potential of a memory leak. You could simply make a copy – but you don’t even need to do that. Just work on the original data.

Next, about naming; it’s general convention in C++ to use underscore_separated_names rather than camelCase or PascalCase. This is only a convention but I’d follow it unless there’s a very compelling reason not to.

Also on the topic of naming: returnSize is redundant, return has no place in the name. Rename it to object_size or something along those lines. You are also inconsistent in your naming convention here.

To get the size of a zero-terminated string, use std::strlen, that’s more efficient than converting to a std::string.

Finally, the code simply doesn’t work. It creates a bit-by-bit shallow representation. That only works for simple composite objects, it no longer works for objects that use pointers or references internally – you’ve already noticed that, because otherwise you wouldn’t need to create a special chase for char* and std::string. Incidentally, for the case of char* you specialise returnSize but for the case of std::string you specialise Serialize and DeSerialize. That’s asymmetrical and means that you have several places that might require changing when new types are added. You should reduce this to a single point which requires specialisation.

Ignoring that, I’d rewrite the code as follows:

#include <algorithm>
#include <iostream>
#include <string>
#include <vector>

typedef unsigned char byte_t;
typedef std::vector<byte_t> buffer;

std::size_t object_size(const char* s) {
    return std::strlen(s);
};

template<typename T>
std::size_t object_size(T const& obj) {
    return sizeof(obj);
};

template<typename T>
buffer serialize(const T& obj) {
    std::size_t size = object_size(obj);
    buffer buf(size);

    byte_t const* obj_begin = reinterpret_cast<byte_t const*>(&obj);
    std::copy(obj_begin, obj_begin + size, buf.begin());

    return buf;
};

template<>
buffer serialize<std::string>(std::string const& str) {
    return serialize(str.c_str());
};

template<typename T>
T deserialize(buffer const& buf) {
    return *reinterpret_cast<const T*>(&buf[0]);
}

template<>
std::string deserialize<std::string>(buffer const& buf) {
    return deserialize<char*>(buf);
}

int main() {
    using std::cout;

    int x = 97;
    buffer c = serialize(x);
    cout << deserialize<int>(c) << "\n";

    char g = 'g';
    buffer c2 = serialize(g);
    cout << deserialize<char>(c2) << "\n";

    std::string k = "blabla";
    buffer c3 = serialize(k.c_str());
    cout << deserialize<char*>(c3) << "\n";

    std::string ka = "string";
    buffer c4 = serialize(ka);
    cout << deserialize<std::string>(c4) << "\n";
}

… but like I said, this solution doesn’t generalise and is pretty useless in practice. Have a look at Boost.Serialization for a proper implementation. Unfortunately this is quite a complex problem.

Answer 2

I'll expand on the portability aspect that wasn't quite explained in the other answer:

Be aware that _tmain(), _TCHAR, and system("PAUSE") are specific to Windows environments, while the first two are also Microsoft extensions that are not considered standard C++.

This can be fixed by changing

• _tmain() to main()
• _TCHAR to char

• system("PAUSE") to something like std::cin.get()

  However, if you can get your IDE to pause automatically, then you won't need your own.