CODEWITHSUNDEEP
c++c++11vectorcopy-constructorifstream
gimiki
gimiki

Reputation: 93

C++ vector of objects of custom class - copy constructor deleted - std::ifstream

I'm trying to create a vector of objects of a custom class A:

class A {
    std::ifstream _file;
    std::string _fileName;
public:
    A(std::string &fileName) : _fileName(fileName) {
        this->_file = std::ifstream(this->_fileName, std::ios::in);
    }
    ~A() {
        this->_file.close();
    }
};

In the main I'm pushing n objects of class A with a for loop iterating a vector of file names.

Example:

#include <iostream>
#include <string>
#include <vector>
#include "A.hpp"

int main() {

    std::vector<A> AList;
    std::vector<std::string> fileList = { "file1", "file2", "file3" };

    for (auto &f : fileList) {
        std::cout << f << std::endl;
        A tempObj(f);
        AList.emplace_back(tempObj);
    }

    return 0;
}

But I get this error: /usr/include/c++/9.1.0/bits/stl_construct.h:75:7: error: use of deleted function ‘A::A(const A&)’

If I'm not wrong, since I have a member std::ifstream inside my class A, the copy constructor is deleted (Ref: https://en.cppreference.com/w/cpp/io/basic_ifstream/basic_ifstream)

How can I solve it? What I'm doing wrong?

Thanks for your help

Upvotes: 2

Views: 1930

Answers (3)

gimme_danger
gimme_danger

Reputation: 798

If I'm not wrong, since I have a member std::ifstream inside my class A, the copy constructor is deleted (Ref: https://en.cppreference.com/w/cpp/io/basic_ifstream/basic_ifstream)

You are right, std::basic_ifstream is an example of uncopyable type, because it's illogical to have several copies of a single stream. In this situation you should use move semantics. Just add move constructor to your class definition:

A(A&& other) noexcept 
  : _file(move(other._file))
  , _fileName(move(other._fileName))
{}

And now your code works fine, because vector elements are properly constructed during pushing:

std::vector<A> AList;
for (auto &f : { "file1", "file2", "file3" })
  AList.push_back(A(f));

Upvotes: 1

HolyBlackCat
HolyBlackCat

Reputation: 96336

As noted in the other answer, class A is non-copyable because its member std::ifstream _file; is non-copyable.

But std::ifstream is movable, and normally your class would be movable as well, but providing a custom destructor prevents the implicit generation of a move constructor and a move assignment operator (see the chart here).

So, step 1: Make your class movable by removing the custom destructor (the implicitly generated destructor will do the same thing anyway).

Alternatively, if you want to keep the destructor for some reason, you need to ask your compiler to generate a move constructor and a move assignment operator:

A(A &&) = default;
A &operator=(A &&) = default;

Step 2: When adding an instance of class A to the vector, move it instead of copying:

A tempObj(f);
AList.emplace_back(std::move(tempObj));
//                 ^^^^^^^^^^       ^

Alternatively, construct it directly in the vector:

AList.emplace_back(f);

Upvotes: 2

Remy Lebeau
Remy Lebeau

Reputation: 596672

Like you said, your A class is not copyable due to the ifstream member, which can't be copied. So your class's copy constructor is deleted by default. But you are trying to copy-construct an A object when you pass tempFile to emplace_back().

You need to pass the filename instead to emplace_back() and let it construct the A object inside the vector for you by forwarding the string to your constructor:

std::vector<A> AList;
std::vector<std::string> fileList;

for (auto &f : fileList)
{
    AList.emplace_back(f);
}

On a side note, your constructor can and should initialize the ifstream in the member initialization list rather than in the constructor body:

A::A(std::string &fileName)
    : _file(fileName), _fileName(fileName)
{
}

Upvotes: 5

Related Questions