Thread: Passing ifstream object between functions

You are returning a reference to a local variable. That is a Bad Thing. I suggest that you just get rid of the inputbuffer function, e.g., write:

Code:

ifstream inputfile("C:\\Product Description.txt");
lexer(inputfile);

Originally Posted by Vespasian

1) Instead of returning by reference, I return a pointer. I guess this doesn't help at all does it, because the pointer will still try point to a local variable that's out of scope.

Indeed, returning a pointer to a local variable has the same problem: the object no longer exists once the function goes out of scope.

Originally Posted by Vespasian

2) What about allocating the inputfile object into the heap, through new/malloc, then returning a reference or pointer to it?

I wouldn't do that (and malloc should not be used here, except in special cases). Creating the object while opening the file is as easy as I have demonstrated (minus error handling). If there is something complicated in say, constructing the file name that should go into a function, write a function to return the file name.

I know that heap allocation of ifstream is advised against, but for the sake of education, I tried allocating using new[] and using pointer to the newly allocated memory as seen below:

Code:

void lexer(ifstream* inputfile)
{
    string line;
    getline(*inputfile,line);
}

ifstream* inputbuffer()
{
    ifstream* inputfileptr = new ifstream[10];
    ifstream inputfile;
    inputfile.open ("C:\\Product Description.txt");
    *inputfileptr = inputfile;
    return inputfileptr;
}

int main()
{
    lexer(inputbuffer());
    return 0;
}

I get a compiler error. The error message mentions:

'std::ios_base& std::ios_base:perator=(const std::ios_base&)' is private

It doesnt mention what line it is but actually opens up some very long 1000 line .cpp file that I assume is the class code for ifstream. When I blank out line 12, it seems to compile so I assume line 12 is the problem.

ifstream objects are not copyable. Even if they were, it would make no sense to create ten of them when you only need one, and then create yet another one to assign to one of the ten. Oh, and you did not use delete (or rather delete[], in your case).

You are probably after something like:

Code:

void lexer(ifstream* inputfile)
{
    string line;
    getline(*inputfile, line);
}

ifstream* inputbuffer()
{
    return new ifstream("C:\\Product Description.txt");
}

int main()
{
    ifstream* inputfile = inputbuffer();
    lexer(inputfile);
    delete inputfile;
    return 0;
}

Originally Posted by Vespasian

My understanding of new[] is that one needs to allocate sized chunks of memory, say, ifstream [10].

That would create 10 ifstream objects. If you want just one:

Code:

return new ifstream;

Originally Posted by Vespasian

So Im kinda confused why theres a string in there?

The same reason why I wrote:

Code:

ifstream inputfile("C:\\Product Description.txt");

instead of:

Code:

ifstream inputfile;

The string is there because it is the argument to the relevant constructor.

Then, remember to delete what you new, and delete[] what you new[]. Then again, no: remember not to use new or new[] unless you are going to use it with a smart pointer, or are implementing at a lower level. Rather, use containers where appropriate, or in this case, just don't use new since you don't need it.

In C++11, we have yet another option, which is moving the object:

Code:

void lexer(ifstream&& inputfile)
{
    string line;
    getline(inputfile,line);
}
 
ifstream inputbuffer()
{
    ifstream inputfile("C:\\Product Description.txt");
    return inputfile;
}
 
int main()
{
    lexer(inputbuffer());
    return 0;
}

This is a little trickier, but the essence is that we take the state of the inputfile and move it from inputfile into a temporary constructed ifstream which is returned.
Then--and here's another tricky thing--we pass that temporary to lexer. However, the problem is that we just got a temporary (put simply, a variable that does not have a name and have limited lifetime). A reference (ifstream&) cannot accept a temporary according to the rules, so we have to use an r-value reference, which essentially is a reference to a temporary.
Now it works as expected, without the use of new and sidestepping your problems.

It is a little more complex, though, and laserlight's solution of constructing the stream and passing it via reference works, too, and is probably a little easier.