5
\$\begingroup\$

Did I cover all the bases? Can this be improved (best practice)?

Edit... I've come up with some improvements. When I tried edit the code, the software complained about too much code and too little blather (paraphrasing). So, here I am blathering. When I have blathered enough, the code below will be the new stuff.

//#include <libdj/istream.h>

#include <istream>
#include <string>
#include <stdexcept>

namespace dj {
    inline
    void append_istream_onto_string(std::istream& inp, std::string &outp) {
        // Uses inp.seekg(), and inp.tellg() to determine size
        // Throws (std::) invalid_argument, runtime_error, bad_alloc, length_error
        if (!inp.good()) {
            throw std::invalid_argument
              ("Input stream is not valid. (read_istream_into_string)");
        }
        if (!inp.seekg(0, std::ios::end)) {
            throw std::runtime_error
            ("Cannot deduce length of input stream. (read_istream_into_string)");
        }
        outp.reserve(outp.size() + inp.tellg()); // Throws bad_alloc and length_error
        if (!inp.seekg(0, std::ios::beg)) {
            throw std::runtime_error
              ("Input stream seek failed. (read_istream_into_string)");
        }
        outp.append((std::istreambuf_iterator<char>(inp)),
            std::istreambuf_iterator<char>());
    }

    inline
    std::string to_string(std::istream& inp) {
        std::string ret;
        append_istream_onto_string(inp, ret);
        return ret;
    }
}
#include <fstream>

int main() {

    std::ifstream inp("junk.txt");

    std::string buffer;
    buffer = "// junk.txt\n";
    try {
        //dj::append_istream_onto_string(inp, buffer);
        buffer = dj::to_string(inp);
    } catch (const std::exception &ex) {
        std::cout << ex.what() << std::endl;
    }
    std::cout << buffer << std::endl;
}
\$\endgroup\$
1
\$\begingroup\$

Streams are awful. Don't use them. (Joking, but I wish I had a viable alternative to recommend for basic file input. C++ is pathetic in this area.)

  • [code:] tellg() returns a signed type, with -1 as an error value. You might like to check that, and then static_cast it before using it in reserve().

  • [code:] Using std::string::resize() and std::istream::read() to copy from the stream might be faster than using std::string::append() with std::istreambuf_iterators.

  • [design(bug?):] Seeking to the end of the file like that may not work. On Windows, for example, if a file is opened in text mode, the ctrl-z (alt026) character is treated as the end of the file (even though it isn't). As I understand it, you have to use ignore() to find the real end of the file instead.

  • [design:] I doubt it's worth it to create a function to append a string directly. Loading into a string, then appending is probably fast enough. (What if you want to prepend or insert... suddenly you need more functions that basically do the same thing).

  • Write unit tests! Something like this needs a whole bunch of tests that save a string to a file, then load a string and compare the results (binary mode, text mode, special characters, etc.).

The whole article in the link above is useful, but the summary section at the end describes 3 reasonable alternatives for reading from a file stream:

  • Put the istream's rdbuf into a stringstream, then use the .str() function to get a string. This involves extra copying, but is simplest, and should be quick enough.
  • Use .ignore() to find the end of the file, then read the whole thing at once. No extra copies in memory (good for large files), but involves reading the file twice.
  • Read chunks into a std::deque and then copy. Works where it's not possible to seek in the stream.
\$\endgroup\$
  • \$\begingroup\$ Good stuff. ... \$\endgroup\$ – Jive Dadson Mar 3 '18 at 17:23
  • \$\begingroup\$ I have hated std::streams since before they were made standard. I intend to act upon your criticism. I appreciate it enormously. Watch this space. \$\endgroup\$ – Jive Dadson Mar 4 '18 at 9:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.