Is my fstream bad or not good()?

Question

So I have a .cpp file with a Function which recieves a filename, and should return a String with the contents of the file (actualy modified contents, I modified the code to make it more understandable, but that doesn't have any effect on my problem). The problem is that f.good() is returning false and the loop, which reads the file is not working. CODE :

#include "StdAfx.h"
#include "Form21.h"
#include <string>
#include <fstream>
#include <iostream>



    string ReadAndWrite(char* a){
    char filename[8];
    strcpy_s(filename,a);
    string output;
    char c;
    ifstream f(filename,ios::in);
    output+= "Example text"; // <-- this writes and returns just fine!
    c = f.get();

    while (f.good())
      { 

    output+= c;
    c= f.get();         
          }

    return output;
}

Does anyone have an idea on why this is happening? Does it have something to do with, that this is a seperate .cpp file( it doesnt even throw out an error when I remove #include <fstream>). Maybe there is a different kind of method to make the loop? I'll be very happy to hear any suggestions on how to fix this or maybe a different method on how to achieve my goal.

Answer 1

First, there's really no reason to copy the file name you receive -- you can just use it as-is. Second, almost any loop of the form while (stream.good()), while (!stream.bad()), while (stream), etc., is nearly certain to be buggy. What you normally want to do is check whether reading some data worked.

Alternatively, you can skip using a loop at all. There are a couple of ways to do this. One that works nicely for shorter files looks like this:

string readfile(std::string const &filename) { 
    std::ifstream f(filename.c_str());   
    std::string retval;

    retval << f.rdbuf();
    return retval;
}

That works nicely up to a few tens of kilobytes (or so) of data, but starts to slow down on larger files. In such a case, you usually want to use ifstream::read to get the data, something along this general line:

std::string readfile(std::string const &filename) {
    std::ifstream f(filename.c_str());

    f.seekg(0, std::ios_base::end);
    size_t size = f.tellg();

    std::string retval(size, ' ');
    f.seekg(0);
    f.read(&retval[0], size);
    return retval;
}

Edit: If you need to process the individual characters (not just read them) you have a couple of choices. One is to separate it into phases, where you read all the data in one phase, and do the processing in a separate phase. Another possibility (if you just need to look at individual characters during processing) is to use something like std::transform to read data, do the processing, and put the output into a string:

struct character_processor { 
    char operator()(char input) { 
        // do some sort of processing on each character:
        return ~input;
    }
};

std::transform(std::istream_iterator<char>(f),
               std::istream_iterator<char>(),
               std::back_inserter(result),
               character_processor());