6
\$\begingroup\$

I wrote this pretty straight forward begin and end overloads for istream_iterator.

Is this standards compliant? Any problems with usage?

#include <iostream>
#include <iterator>
#include <fstream>

namespace std
{

    template <typename T>
    std::istream_iterator<T> begin(std::istream_iterator<T> iter)
    {
        return iter;
    }

    template <typename T>
    std::istream_iterator<T> end(std::istream_iterator<T>)
    {
        return std::istream_iterator<T>();
    }

}

int main()
{
    for (std::string s : std::istream_iterator<std::string>(std::cin))
    {
        std::cout << s;
    }
}
\$\endgroup\$
  • \$\begingroup\$ I would give it the benefit of the doubt. @user84832, could you please add some clarification as to what exactly you would like reviewed about your code? \$\endgroup\$ – Kaz Sep 18 '15 at 19:46
  • \$\begingroup\$ I see this as a code review. Nice little utility class (I have something similar in my toolbox). \$\endgroup\$ – Martin York Sep 18 '15 at 22:07
3
\$\begingroup\$

Not sure that's how I would have written in.
Its not normal to call std::begin() and std::end() on an iterator.

I would have arranged for a some type to represent the stream as a container:

#include <iostream>
#include <iterator>
#include <fstream>

namespace ThorsAnvil
{
    template<typename T>
    struct StreamAsContainer
    {
        std::istream& str;
        StreamAsContainer(std::istream& str) : str(str) {}
    };

    template <typename T>
    std::istream_iterator<T> begin(StreamAsContainer<T>& c)
    {
        return std::istream_iterator<T>(c.str);
    }

    template <typename T>
    std::istream_iterator<T> end(StreamAsContainer<T>&)
    {
        return std::istream_iterator<T>();
    }

}

int main()
{
    namespace TA=ThorsAnvil;
    for (std::string s : TA::StreamAsContainer<std::string>(std::cin))
    {
        std::cout << s;
    }
}
\$\endgroup\$
2
\$\begingroup\$

Is this standards compliant? Any problems with usage?

No it's not standards compliant. From [namespace.std]:

The behavior of a C++ program is undefined if it adds declarations or definitions to namespace std or to a namespace within namespace std unless otherwise specified. A program may add a template specialization for any standard library template to namespace std only if the declaration depends on a user-defined type and the specialization meets the standard library requirements for the original template and is not explicitly prohibited.

You're adding a declaration to namespace std which does not fall into any of the "otherwise specified" categories (e.g. you're not specializing a template based with a user-defined type). What you are doing is undefined behavior.


To avoid that, you could just wrap the istream in a user-defined container:

template <typename T>
struct istream_range
{
    std::istream& stream;
    std::istream_iterator<T> begin() { return {stream}; }
    std::istream_iterator<T> end() { return {}; }
};

for (std::string s : istream_range<std::string>{std::cin})
{
    std::cout << s;
}

In Eric Niebler's range library, he introduced a way to make an istream range to solve the same problem, but actually introduces different types for the iterator and the sentinel. There, you could also do:

#include <range/v3/istream_range.hpp>
#include <range/v3/algorithm/for_each.hpp>

ranges::for_each(
    ranges::istream<std::string>(std::cin),
    [](std::string const& s){
        std::cout << s;
    });
\$\endgroup\$
  • \$\begingroup\$ I would have done the same as you with the istream_range with begin/end as methods. But I was unsure on the requirements of container like object and what they need to support for std::begin() and std::end() to work correctly. Do you have any incite on using this (admittedly nicer) technique over the free standing functions I use in my answer. \$\endgroup\$ – Martin York Sep 21 '15 at 20:47
1
\$\begingroup\$

Serious problem:

As an user of the language you're not supposed to extend the Standard namespace at will. This makes your code not portable and very fragile. If the Standard Library adds such functionality at some point, your code will likely break.


A very tiny thing, but here:

for (std::string s : std::istream_iterator<std::string>(std::cin))

Is a place where you can and should be using auto. We can see that this piece of code repeats itself by specifying the type std::string twice, thus it should be DRYed up:

for (auto s : std::istream_iterator<std::string>(std::cin))
\$\endgroup\$

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.