views:

103

answers:

3

Hello. I've got a problem with this code:

#include <fstream>

struct A
{   
    A(std::ifstream input)
    {
        //some actions
    }
};

int main()
{
    std::ifstream input("somefile.xxx");

    while (input.good())
    {
        A(input);
    }

    return 0;
}

G++ outputs me this:

$ g++ file.cpp
file.cpp: In function `int main()':
file.cpp:17: error: no matching function for call to `A::A()'
file.cpp:4: note: candidates are: A::A(const A&)
file.cpp:6: note:                 A::A(std::ifstream)

After changing it to this it compile (but that is not solving the problem):

#include <fstream>

struct A
{   
    A(int a)
    {
        //some actions
    }
};

int main()
{
    std::ifstream input("dane.dat");

    while (input.good())
    {
        A(5);
    }

    return 0;
}

Can someone explain me what's wrong and how to fix it? Thanks.

+1  A: 

ifstream does not have a copy constructor. A(std::ifstream input) means "constructor for A taking an ifstream by value." That requires the compiler to make a copy of the stream to pass to the constructor, which it can't do because no such operation exists.

You need to pass the stream by reference (meaning, "use the same stream object, not a copy of it.") So change the constructor signature to A(std::ifstream& input). Note the ampersand, which means "reference" and, in the case of function parameters, means "pass this parameter by reference rather than by value.


Stylistic note: The body of your while loop, A(input);, constructs a structure of type A, which is then almost immediately destroyed when the while loop loops. Are you sure this is what you want to do? If this code is complete, then it would make more sense to make this a function, or a member function of A that is constructed outside the loop:

static void process(std::istream& stream)
{
    // some actions
    // note stream is declared as std::istream&; this lets you pass
    // streams that are *not* file-based, if you need to
}

int main()
{
    std::ifstream input("somefile.xxx");

    while (input.good())
    {
        process(input);
    }

    return 0;
}

OR

struct A
{   
    A()
    {
        // default constructor for struct A
    }

    void process(std::istream& stream)
    {
        // some actions
    }
};

int main()
{
    std::ifstream input("somefile.xxx");

    A something;
    while (input.good())
    {
        something.process(input);
    }

    return 0;
}
Jonathan Grynspan
+4  A: 

Two bugs:

  • ifstream is not copyable (change the constructor parameter to a reference).
  • A(input); is equivalent to A input;. Thus the compiler tries to call the default constructor. Wrap parens around it (A(input));. Or just give it a name A a(input);.

Also, what's wrong with using a function for this? Only the class's constructor is used it seems, which you seem to abuse as a function returning void.

Johannes Schaub - litb
Is not A(input) constructing a temporary A object?
Martin York
@Martin The parentheses around `input` are the binding parentheses used in declarators, for example as they occur in `void (*p)();` around `*p`. The ones around `input` are redundant. This is what people occasionally experience in `vector<int> p(istream_iterator<int>(cin), ...);`, where redundant parentheses are specified around `cin`.
Johannes Schaub - litb
I forgot to mention that I tried that thing with references. (A(input)) was bug, thanks.
darvan
+1  A: 

Streams are non copyable.

So you need to pass by reference.

struct A
{   
    A(std::ifstream& input)
                 ^^^^^
    {
        //some actions
    }
};
Martin York