6
\$\begingroup\$

Exercise:

Using a read7() method that returns 7 characters from a file, implement readN(n) which reads n characters.

For example, given a file with the content "Hello world", three read7() returns "Hello w", "orld" and then "".

I made a version using std::deque. It seems inefficient to me with all the loops to read/write a string, but I couldn't think of an alternative for this FIFO. Any comments on that part would be especially appreciated.

#include <iostream>
#include <sstream>
#include <deque>

inline auto read7()
{
  static std::istringstream is("1234567890123456789012345678901234567890");
  char res[8]{};
  is.get(res, 8);
  return std::string(res);
}

template <typename T1, typename T2>
auto& operator<<(std::deque<T1>& deq, const T2& seq)
{
  for (const auto& elm : seq)
  {
    deq.push_back(elm);
  }
  return deq;
}

template <typename T1, typename T2>
auto get(std::deque<T2>& deq, std::size_t n)
{
  T1 res;
  while (res.size() != n && !deq.empty())
  {
    res += deq.front();
    deq.pop_front();
  }
  return res;
}

auto readN(std::size_t n)
{
  static std::deque<char> buffer;
  while (n > buffer.size())
  {
    auto old_sz = buffer.size();
    buffer << read7();
    if (old_sz == buffer.size())
      break;
  }

  return get<std::string>(buffer, n);
}


int main()
{
  std::size_t sz;
  while(std::cin >> sz)
    std::cout << readN(sz) << '\n';

  return 0;
}
\$\endgroup\$
1
\$\begingroup\$

Pedantically, none of the included headers are guaranteed to define std::size_t. It is (in principle) possible to implement those headers without defining that type, and we're required to include one of the headers that does define it (e.g. <cstddef>).

We could avoid the push_back loop by using the std::copy() algorithm with a std::back_inserter as the destination. More simply, we could just use the deque's insert() like this:

template <typename T1, typename T2>
auto& operator<<(std::deque<T1>& deq, const T2& seq)
{
    using std::begin();
    using std::end();

    deq.insert(end(deq), begin(seq), end(seq));
    return deq;
}

I think the old_sz check in readN() is less clear than simply checking whether the input chunk is an empty string:

std::string s;
while (buffer.size() < n && !(s = read7()).empty()) {
    buffer << std::move(s);
}

Or, just inline the <<, given that it's a one-liner and not used anywhere else:

using std::begin();
using std::end();

std::string s;
while (buffer.size() < n && !(s = read7()).empty()) {
    buffer.insert(end(buffer), start(s), end(s));
}

On the choice of approach: I probably wouldn't use a deque. We only need to store a surplus of up to 7 characters, which is fine to hold in a string (most implementations use a "small-string" optimisation to avoid allocating heap space for the characters). I would implement readN() by starting with an empty, n-capacity string and building it up in place, storing the new surplus back to the string where we keep it.

\$\endgroup\$
3
\$\begingroup\$

Why is read7 declared inline? You should only do this when it's needed as an optimization. (And it's particularly confusing when the function contains a static variable.)

It's not necessary for is to be static. It should really be an argument to read7, so it can work on any istream. (In real code, buffer should also be a parameter to readN, but for this toy problem it's reasonable to have it static.)

Bug: what happens if the input contains null characters?

You could avoid the queue by using a string as the queue. You can simply append the result of each read7 to it until you have enough characters, then remove and return the first n.

\$\endgroup\$
  • \$\begingroup\$ is is static because read7 is supposed to be that magic function that gives you 7 characters. Maybe like std::getline \$\endgroup\$ – Ayxan Sep 5 at 21:08
  • 1
    \$\begingroup\$ Removing first n characters from string is really inefficient, isn't it? How is this an improvement \$\endgroup\$ – Ayxan Sep 5 at 21:12
  • \$\begingroup\$ @Ayxan Making it an argument allows you to read from any stream, for example a file. \$\endgroup\$ – L. F. Sep 6 at 10:20
  • \$\begingroup\$ @L.F. read7 is not a part of the exercise. It's a "magic" function that returns 7 characters. If I had the stream in the first place, I would read n characters without the need of readN \$\endgroup\$ – Ayxan Sep 6 at 18:02
  • \$\begingroup\$ @Ayxan Oh, now I understand. That's a really interesting exercise :) \$\endgroup\$ – L. F. Sep 7 at 1:24

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.