Iterating over the lines of a stream

This structure is supposed to iterate over the lines of an input stream, using std::getline. Any remark is welcome!

#include  //istream
#include 

struct line_iterator
{
    explicit
    line_iterator( std::istream & is )
        : m_stream( &is )
        , m_line( extract_line() )
    {
    }

    line_iterator()
        : m_stream( nullptr )
        , m_line()
    {
    }

    line_iterator( line_iterator & ) = default;

    std::string operator*()
    {
        assert( m_stream );
        return m_line;
    }

    line_iterator operator++()
    {
        assert( m_stream );
        update_stream();
        return *this;
    }

    line_iterator operator++(int)
    {
        line_iterator copy( *this );
        ++(*this);
        return copy; 
    }
private:
    void update_stream()
    {
        if ( m_stream->good() )
            m_line = extract_line();

        else
        {
            m_line.clear();
            m_stream = nullptr;
        }
    }

    std::string extract_line()
    {
        std::string line;
        std::getline( *m_stream, line );
        return line;
    }

private:
    std::istream *  m_stream;
    std::string     m_line;

    friend bool operator!=( const line_iterator&, const line_iterator& );
    friend bool operator==( const line_iterator&, const line_iterator& );
};

inline
bool operator!=( const line_iterator & a, const line_iterator & b )
{
    return !(a.m_stream == b.m_stream);
}

inline
bool operator==( const line_iterator & a, const line_iterator & b )
{
    return a.m_stream == b.m_stream;
}

-
You might consider providing typedefs for the types expected by std::iterator_traits.

-
Your type should meet the requirements of InputIterator. This means that you should also provide an overload for operator ->.

-
The semantics of operator != are not really useful. For example, this won't allow you to use your iterator in the idiomatic

line_iterator begin = …;
line_iterator end = …;
while (begin != end)
  {
    std::cout << *begin << std::endl;
    ++begin;
  }

because there is no value you can set end to such that it compares not non-equal to begin only after begin was advanced a certain number of times. I suggest you implement it somehow like this:

friend bool
operator!=(const line_iterator& lhs, const line_iterator& rhs) noexcept
{
  if (lhs.m_stream == nullptr && rhs.m_stream == nullptr)
    return false;
  else if (lhs.m_stream == nullptr)
    return rhs.m_stream->good();
  else if (rhs.m_stream == nullptr)
    return lhs.m_stream->good();
  else
    return lhs.m_stream != rhs.m_stream;
}

This will allow you to use a default-constructed line_iterator as end and have it compare not non-equal to begin only after begin's stream is exhausted.

You should then implement lhs == rhs as !(lhs != rhs).

-
You are making many copies of the string which is not as efficient as it could be. I also feel that it makes your code more complicated than necessary. You should read directly into m_string by implementing the increment operator as

line_iterator&
operator++()
{
  assert(m_stream && m_stream->good());
  std::getline(*m_stream, m_string);
  return *this;
}

and the dereferencing operator as

const std::string&
operator*() const
{
  return m_string;
}

In your constructor, default-construct m_string and (if the pre-conditions are met) advance once.

-
Personally, I prefer to only mention those members in the constructor's initializer list that depend on the constructor arguments. Those that get default values I prefer to initialize in the class definition.

class line_iterator
{

public:

  line_iterator()
  {
    // nothing to do
  }

  line_iterator(std::istream& istr) : m_stream {&istr}
  {
    if (m_istr->good())
      std::getline(*m_stream, m_string);
  }

  …

private:

  std::string m_string {};     // defaults to empty string
  std::istream * m_stream {};  // defaults to 'nullptr'
};

-
If you're going to make the friends inline anyway, I'd prefer to define them right in the class body.

StackExchange Code Review Q#84981, answer score: 2