Re: [boost] [gil::io] Feedback for scanline_read_iterator

Subject: Re: [boost] [gil::io] Feedback for scanline_read_iterator
From: Jeffrey Lee Hellrung, Jr. (jeffrey.hellrung_at_[hidden])
Date: 2013-02-18 20:46:59

On Mon, Feb 18, 2013 at 5:03 PM, Christian Henning <chhenning_at_[hidden]>wrote:

> >
> > So...doing 2 operator++'s in a row isn't logically equivalent?
>
> Right now, operator++ only sets the _read_scanline flag. So, doing 2
> operator++ wont do anything different.
>
> Do have something in mind?
>
> Here is a link to the current version.
>
>
> http://svn.boost.org/svn/boost/trunk/boost/gil/extension/io/detail/scanline_read_iterator.hpp

My first reaction is, it doesn't look like the copy constructor will do the
right thing (or is it okay to do _reader->clean_up() multiple times?).

Secondly, the behavior of operator* and operator++ doesn't read right, but
I'm not intimately familiar with what the implementation is suppose to do.
Here they are (simplified and reformatted):

reference operator*() {
  if( _read_scanline ) {
    _reader->read( _buffer, _pos );
  } else {
    _reader->skip( _buffer, _pos );
  }
  ++_pos;
  _read_scanline = false;
  return _buffer;
}

/// Pre-Increment Operator
scanline_read_iterator& operator++() {
  _read_scanline = true;
  return *this;
}

So, operator* effectively advances the iterator, while
operator++...doesn't? Huh? (Do I have that transcribed correctly?)

What I *think* you want is for operator++ to conditionally call
_reader->skip (if the previous operator++ had not been followed by a
operator*), while operator* should conditionally call _reader->read
(depending on if there had been an intervening operator++ after the
previous operator*). So, I think you'd need 2 boolean flags.

Does that make sense? (I might objectively not be making sense, I'm not
sure.)

- Jeff