'How do I use move-semantics to reallocate resources from class constructor?

To enable me to read a binary file I have made this class. I would like to not copy a temporary for which I am trying to use move semantics. But this code produces "access violation error".

#include <fstream>
#include <iostream>

class myClass
{
public:
    std::istream& myStream;
    static myClass create(const char* path);
    myClass(myClass&&) noexcept = default;
    myClass(std::istream& input);
    myClass() = default;
};

myClass myClass::create(const char* path)
{
    std::ifstream input{ path, std::ios::binary | std::ios::in };
    if (!input.is_open()) throw std::runtime_error("Error - couldn't open file");
    return std::move(myClass(input));
}

myClass::myClass(std::istream& input) :
    myStream(input)
{

}
int main()
{
    const char* path = R"(file.bin)";
    myClass cr{myClass::create(path)};

    for (int i = 0; i < 10; ++i)
        cr.myStream.seekg(i);

    return 0;
}


Solution 1:[1]

You are accessing the stream after the temporary has been destroyed. You need to store the stream in the myClass object:

#include <fstream>
#include <iostream>

class myClass
{
public:
    std::istream& myStream;
    static myClass create(const char* path);
    myClass(myClass&& other) noexcept;
    myClass(std::istream& input);
    myClass() = default;
};

myClass myClass::create(const char* path)
{
    std::ifstream input{ path, std::ios::binary | std::ios::in };
    if (!input.is_open()) throw std::runtime_error("Error - couldn't open file");
    return std::move(myClass(input));
}

myClass::myClass(myClass&& other) noexcept :
    myStream(other.myStream)
    {
    other.myStream = std::cin;
}

myClass::myClass(std::istream& input) :
    myStream(input)
{

}
int main()
{
    const char* path = R"(file.bin)";
    myClass cr{myClass::create(path)};

    for (int i = 0; i < 10; ++i)
        cr.myStream.seekg(i);

    return 0;
}

Solution 2:[2]

The problem is the lifetime of input in myClass::create(). As JensenMcKenzie mentioned, it will be destroyed at the end of that function, and therefore the reference passed to the returned myClass object will be a dangling reference. The only way to keep the myClass::create() function would be to store a concrete std::ifstream object inside myClass:

class myClass
{
    std::ifstream myStream;
    ...
};

To allow another std::ifstream to be passed to the constructor, it has to be as an r-value reference:

myClass::myClass(std::ifstream&& input): myStream(std::move(input)) {}

And then you can write the create() function like so:

myClass myClass::create(const char* path)
{
    std::ifstream input{ path, std::ios::binary | std::ios::in };
    if (!input.is_open()) throw std::runtime_error("Error - couldn't open file");
    return std::move(input);
}

Note that you could also just have written a constructor that takes a pathname as an argument:

myClass::myClass(const char *path):
        myStream(path, std::ios::binary | std::ios::in)
{
    if (!input.is_open())
        throw std::runtime_error("Error - couldn't open file");
}

And then you could have written this in main() instead:

const char *path = ...;
myClass cr(path);

This way no moves are necessary at all.

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1 JensenMcKenzie
Solution 2 G. Sliepen