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-19 23:40:01

On Feb 19, 2013 8:15 PM, "Jeffrey Lee Hellrung, Jr." <
jeffrey.hellrung_at_[hidden]> wrote:
>
> On Tue, Feb 19, 2013 at 5:33 PM, Christian Henning <chhenning_at_[hidden]>
wrote:
>>
>> >
>> > 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.)
>> >
>>
>> I think your thinking makes sense! I have updated the code here:
>>
>>
http://svn.boost.org/svn/boost/trunk/boost/gil/extension/io/detail/scanline_read_iterator.hpp
>>
>> If you could have a look that would be very kind.
>
>
> I had a quick look, and I immediately noticed that you (still, I guess?)
are incrementing the _pos member upon dereferencing the first time...is
there some reason you don't increment it unconditionally in operator++? I'm
guessing the end result is the same, given the pair of boolean flags
encoding the state, but it's a "code surprise", which increases the
cognitive overhead to read it :)
>
> - Jeff
>

Oh, and after thinking about this a little, effectively incrementing your
position within operator* will make operator== behave strangely :/

- Jeff