14
\$\begingroup\$

I need to read n chars from a binary file into a string. Currently, what I do is:

static string Read(istream &stream, uint32_t count)
{
    auto bytes = unique_ptr<char[]>(new char[count]);

    stream.read(bytes.get(), count);

    return string(bytes.get(), count);
}

I found the way I deal with the array of chars quite messy. If I used new and delete[] directly, it would make the code messy in another way (I would need to add a local variable for the result). And I'm trying to avoid delete as a general rule.

Is there a clear way to write this code? The fact that it uses twice as much memory as it needs is probably not a big deal, but fixing that would be nice too.

\$\endgroup\$
3
  • \$\begingroup\$ I think this is actually quite clean – albeit low-level – code. Nothing really wrong with it, and it’s probably more efficient than ruds’ implementation, although that’s arguably even cleaner. \$\endgroup\$ Jul 20, 2013 at 13:51
  • \$\begingroup\$ @KonradRudolph: There is no need to be calling new here. Especially since the string object will deal with all that for you. \$\endgroup\$ Jul 20, 2013 at 21:48
  • \$\begingroup\$ @Loki Ah true, I’d completely forgotten about C++11’s contiguity requirement. \$\endgroup\$ Jul 20, 2013 at 22:03

2 Answers 2

21
\$\begingroup\$

Why not:

static string Read(istream &stream, uint32_t count)
{
    std::string result(count, ' ');
    stream.read(&result[0], count);

    return result;
}

Though not strictly C++03 compatible that is easily validated. One of the reasons the committee found it easy to add the new constraint in C++11 was that no implementation did not use contiguous memory (Random Access Iterators are a hint).

But a C++03 strict implementation would be:

static string Read(istream &stream, uint32_t count)
{
    std::vector<char> result(count);  // Because vector is guranteed to be contiguous in C++03
    stream.read(&result[0], count);

    return std::string(&result[0], &result[count]);
}
\$\endgroup\$
3
  • \$\begingroup\$ Yeah, that's certainly better. \$\endgroup\$
    – svick
    Jul 20, 2013 at 21:57
  • 3
    \$\begingroup\$ Note that this requires C++11 in a non-obvious way (C++03 didn't require that &result[0] worked this way). Otherwise, LGTM. \$\endgroup\$
    – ruds
    Jul 20, 2013 at 22:12
  • \$\begingroup\$ Depending on your use case, you may want to do result.resize(stream.gcount());, in case not all of count bytes could be read. \$\endgroup\$ Apr 21, 2015 at 18:40
8
\$\begingroup\$

First of all, it seems that you've got a using namespace std; somewhere in your code. Don't do that. (No, really).

Here's a function that should meet your needs.

static std::string Read(std::istream &stream, std::string::size_type count)
{
    std::string out;
    out.reserve(count);
    std::copy_n(std::istreambuf_iterator(stream), count, std::back_inserter(out));
    return out;
}
\$\endgroup\$
10
  • 1
    \$\begingroup\$ uint32_t should have an std:: as well. \$\endgroup\$
    – Jamal
    Jul 19, 2013 at 20:07
  • \$\begingroup\$ I don't have using namespace std; there. Instead, I have using std::uint32_t; using std::unique_ptr; using std::string; using std::istream; there. It's kind of annoying, but I thought it would be better than writing std:: everywhere. \$\endgroup\$
    – svick
    Jul 19, 2013 at 20:12
  • 3
    \$\begingroup\$ @Jamal You're right. Actually, I've changed it to std::string::size_type, as that's the size class we're interested in. \$\endgroup\$
    – ruds
    Jul 19, 2013 at 20:13
  • \$\begingroup\$ @svick I think it's odd to read code with names from std but with no std:: in front. Personal tastes vary, of course. \$\endgroup\$
    – ruds
    Jul 19, 2013 at 20:14
  • \$\begingroup\$ Is this going to insert the characters one by one? I worry that doing that would be too slow (the result might be one megabyte or so). (Of course, the proper way to find that out would be to measure it.) \$\endgroup\$
    – svick
    Jul 19, 2013 at 20:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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