HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

What would be a correct C++ style for function loading file contents?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filewhatfunctioncontentswouldstyleforcorrectloading

Problem

bool loadFileContents(const std::string& fileName, std::string &out) {
    bool res = false;
    FILE* file = fopen(fileName.c_str(), "r");
    char buf[128] = { 0 };
    if (file) {
        while (fread(buf, 1, sizeof(buf), file))
            out += std::string(buf);
        res = true;
    }
    fclose(file);
    return res;
}


The rationale behind using std::string for the out parameter is that it handles the buffer underneath. However, I feel like it's a move from C-style, but not quite a C++ one.

Should I consider using some kind of stream for the out param?

Solution

Here are some of the C-style features of your code:

  • FILE



  • fopen



  • char buf [128]



  • fclose



I personally liked the answer that was given in @Jamal's link.

std::ifstream fin(fileName);
std::ostringstream oss;

oss << fin.rdbuf();
out = oss.str();


The only problem with this is that it doesn't work well with binary files.

You could make out be a std::vector instead.

std::vector  loadFileContents (const std::string &filename)
{
    std::ifstream ifile ;
    ifile.exceptions (std::ifstream::badbit | std::ifstream::failbit) ;

    std::vector  vecBuffer ;
    typedef std::vector ::size_type size_type ;

    try {
        ifile.open (filename) ;
        ifile.seekg (0, ifile.end) ;
        size_type size = static_cast  (ifile.tellg ()) ;
        ifile.seekg (0, ifile.beg) ;
        vecBuffer.resize (size) ;
        ifile.read (&vecBuffer [0], size) ;
    }

    catch (std::ios_base::failure &e) {
        std::cerr << e.what () << std::endl ;
    }

    return vecBuffer ;
}


NRVO allows us not to worry about returning the std::vector by value.

If you have access to C++11, then you could do:

std::vector  loadFileContents (const std::string &filename)
{
    std::ifstream ifile ;
    ifile.exceptions (std::ifstream::badbit | std::ifstream::failbit) ;

    std::vector  vecBuffer ;

    try {
        ifile.open (filename) ;
        std::istreambuf_iterator  iter (ifile) ;
        std::istreambuf_iterator  eos ;
        vecBuffer = std::move (std::vector  (iter, eos)) ;    
    }

    catch (std::ios_base::failure &e) {
        std::cerr << e.what () << std::endl ;
    }

    return vecBuffer ;
}


If you want to enter template territory, then you could try this:

template 
Container loadFileContents (const std::string &filename)
{
    std::ifstream ifile ;
    ifile.exceptions (std::ifstream::badbit | std::ifstream::failbit) ;

    try {
        ifile.open (filename) ;
        std::istreambuf_iterator  iter (ifile) ;
        std::istreambuf_iterator  eos ;
        return Container (iter, eos) ;  
    }

    catch (std::ios_base::failure &e) {
        std::cerr << e.what () << std::endl ;
    }

    return Container () ;
}


Then in your main, you could do:

int main (void)
{
    std::string strData = loadFileContents  (FILENAME) ;
    std::vector  vecData = loadFileContents  > (FILENAME) ;

    return 0 ;
}


If we choose to not return the Container, then we could do:

template 
void loadFileContents (const std::string &filename, Container &container)
{
    std::ifstream ifile ;
    ifile.exceptions (std::ifstream::badbit | std::ifstream::failbit) ;

    try {
        ifile.open (filename) ;
        std::istreambuf_iterator  iter (ifile) ;
        std::istreambuf_iterator  eos ;
        container = Container (iter, eos) ; 
    }

    catch (std::ios_base::failure &e) {
        std::cerr  vecData ;

    loadFileContents (FILENAME, strData) ;
    loadFileContents (FILENAME, vecData) ;

    return 0 ;
}


The benefit of this one is that we wouldn't have to supply template arguments in main.

Code Snippets

std::ifstream fin(fileName);
std::ostringstream oss;

oss << fin.rdbuf();
out = oss.str();
std::vector <char> loadFileContents (const std::string &filename)
{
    std::ifstream ifile ;
    ifile.exceptions (std::ifstream::badbit | std::ifstream::failbit) ;

    std::vector <char> vecBuffer ;
    typedef std::vector <char>::size_type size_type ;

    try {
        ifile.open (filename) ;
        ifile.seekg (0, ifile.end) ;
        size_type size = static_cast <size_type> (ifile.tellg ()) ;
        ifile.seekg (0, ifile.beg) ;
        vecBuffer.resize (size) ;
        ifile.read (&vecBuffer [0], size) ;
    }

    catch (std::ios_base::failure &e) {
        std::cerr << e.what () << std::endl ;
    }

    return vecBuffer ;
}
std::vector <char> loadFileContents (const std::string &filename)
{
    std::ifstream ifile ;
    ifile.exceptions (std::ifstream::badbit | std::ifstream::failbit) ;

    std::vector <char> vecBuffer ;

    try {
        ifile.open (filename) ;
        std::istreambuf_iterator <char> iter (ifile) ;
        std::istreambuf_iterator <char> eos ;
        vecBuffer = std::move (std::vector <char> (iter, eos)) ;    
    }

    catch (std::ios_base::failure &e) {
        std::cerr << e.what () << std::endl ;
    }

    return vecBuffer ;
}
template <typename Container>
Container loadFileContents (const std::string &filename)
{
    std::ifstream ifile ;
    ifile.exceptions (std::ifstream::badbit | std::ifstream::failbit) ;

    try {
        ifile.open (filename) ;
        std::istreambuf_iterator <char> iter (ifile) ;
        std::istreambuf_iterator <char> eos ;
        return Container (iter, eos) ;  
    }

    catch (std::ios_base::failure &e) {
        std::cerr << e.what () << std::endl ;
    }

    return Container () ;
}
int main (void)
{
    std::string strData = loadFileContents <std::string> (FILENAME) ;
    std::vector <char> vecData = loadFileContents <std::vector <char> > (FILENAME) ;

    return 0 ;
}

Context

StackExchange Code Review Q#38784, answer score: 2

Revisions (0)

No revisions yet.