I wrote two functions to save and read data in a bin file:

#include <iostream>
#include <fstream>
#include <vector>

using namespace std;

// save data in file with name p_file
template <typename T>
void save(string p_file, T data) {
    ofstream output(p_file, ios::binary | ios::out);
    output.write((char*) &data, sizeof(data));
    output.close();
}

// read and return data in file with name p_file
template <typename T>
T read(string p_file) {
    ifstream input(p_file, ios::binary | ios::in);
    T data;
    input.seekg(0, input.end);
    int length = input.tellg();
    input.seekg(0, input.beg);
    input.read((char*) &data, length);
    input.close();
    return data;
}

int main() {
    vector<int> vint;
    vint.push_back(1);
    save< vector<int> >("test.bin", vint);
    vector<int> load_vint = read< vector<int> >("test.bin");
    cout << vint[0] << endl;
    cout << load_vint[0] << endl;
    cout << "done" << endl;
}

expected output:

1
1
done

actual output:

1
1990048
done

Things got even worse when I put most of the code in main into test and didn't change anything in save and read:

#include <iostream>
#include <fstream>
#include <vector>

using namespace std;

// save data in file with name p_file
template <typename T>
void save(string p_file, T data) {
    ofstream output(p_file, ios::binary | ios::out);
    output.write((char*) &data, sizeof(data));
    output.close();
}

// read and return data in file with name p_file
template <typename T>
T read(string p_file) {
    ifstream input(p_file, ios::binary | ios::in);
    T data;
    input.seekg(0, input.end);
    int length = input.tellg();
    input.seekg(0, input.beg);
    input.read((char*) &data, length);
    input.close();
    return data;
}

// exact same code, just in a function
void testing() {
    vector<int> vint;
    vint.push_back(1);
    save< vector<int> >("test.bin", vint);
    vector<int> load_vint = read< vector<int> >("test.bin");
    cout << vint[0] << endl;
    cout << load_vint[0] << endl;
}

int main() {
    testing();
    cout << "done" << endl;
}

expected output:

1
1
done

actual output:

1
1924512

What is going on and how do I fix this error?

CodePudding user response：

You are passing the address of the vector object to the save function (which lives on the stack) and not the underlying dynamic array (lives on the heap memory) which holds the ints. Also have a look at how std::vector works: https://www.learncpp.com/cpp-tutorial/an-introduction-to-stdvector/

Here is my full solution with lots of refactoring and cleanup!!

main.cpp

#include <iostream>
#include <fstream>
#include <vector>


// save data in file with name p_file
void save( const std::string& p_file, const std::vector<int>& data )
{
    std::ofstream output( p_file, std::ofstream::binary );

    if ( !output.is_open( ) )
    {
        throw std::ios_base::failure( "Error while opening the file "   p_file );
    }

    for ( size_t idx = 0; idx < data.size( );   idx )
    {
        output.write( reinterpret_cast< const char* >( &(data[idx]) ), sizeof( int ) );
    }

    /*
    if ( !data.empty() )                                                          // Or use this instead of the for loop
    {
        size_t numOfBytes { data.size( ) * sizeof( int ) };
        output.write( reinterpret_cast< const char* >( &(data[0]) ), numOfBytes );
    }
    */

    output.close();
}

// read and return data in file with name p_file
std::vector<int> read( const std::string& p_file )
{
    std::ifstream input( p_file, std::ifstream::binary );

    if ( !input.is_open( ) )
    {
        throw std::ios_base::failure( "Error while opening the file "   p_file );
    }

    input.seekg( 0, input.end );
    size_t length = input.tellg();
    input.seekg( 0, input.beg );

    size_t numOfIntsInFile { length / sizeof( int ) };

    std::vector<int> data( numOfIntsInFile );

    for ( size_t idx = 0; idx < ( numOfIntsInFile );   idx )
    {
        input.read( reinterpret_cast< char* >( &(data[idx]) ), sizeof( int ) );
    }

    /*
    if ( !data.empty() )                                               // Or use this instead of the for loop
    {
        input.read( reinterpret_cast< char* >( &(data[0]) ), length );
    }
    */

    input.close();

    return data;
}

// exact same code, just in a function
void test()
{
    std::vector<int> vint;
    vint.push_back( 1 );
    vint.push_back( 2235 );
    vint.push_back( 3 ); // push back as many ints as you want, it won't break.

    std::vector<int> load_vint;

    try
    {
        save( "test.bin", vint );
        load_vint = read( "test.bin" );
    }
    catch (const std::ios_base::failure& e)
    {
        std::cerr << "Caught an std::ios_base::failure.\n"
                  << e.what() << '\n'
                  << "Error code: " << e.code() << '\n';
    }

    std::cout << '\n';
    std::cout << "Printing the elements of vint: " << '\n';

    for ( const auto& element : vint )
    {
        std::cout << element << '\n';
    }

    std::cout << '\n';
    std::cout << "Printing the elements of load_vint: " << '\n';

    for ( const auto& element : load_vint )
    {
        std::cout << element << '\n';
    }
}

int main()
{
    test();
    std::cout << "\nDone." << std::endl;
}

Summary of the changes:

1. I removed the templates since they were annoying me! You can add them to my code if you really need template functions and then use them.

2. Use lvalue references (like const std::vector<int>&) where possible to avoid unnecessary copies.

3. Use C casts (like reinterpret_cast) instead of C-style casts as much as possible.

4. Avoid polluting the whole source file by writing using namespace std; at the top. Use it in limited scopes.

5. Use std::ofstream::binary or std::ifstream::binary instead of std::ios::binary where dealing with fstream objects.

6. Also added exception handling mechanisms in case a file can not be opened.

Extra note: Cleaning the code and making it a bit more readable ensures that other people can easily read and understand your problem.