6

Assume I have a C++ iterator that not only traverses a data structure but also applies a transformation to the elements when it is dereferenced.

As a real-world example, here's an iterator that goes over the pixels in a bitmap, transforming the bitmap-specific pixel format into a convenient structure:

class ConstPixelIterator {

  public: struct Pixel {
    float Red;
    float Green;
    float Blue;
    float Alpha;
  };

  public: ConstPixelIterator(const Bitmap &bitmap);

  // ...standard iterator functionality...

  public: Pixel operator *() {
    // Read from memory and convert pixel format-specific bytes into Pixel structure
  }

};

Now if I wanted to implement a non-const iterator (i.e. let the user modify pixels), what is the best way to go about this?

Some ideas I considered:

  • I could put accessor methods in the Pixel structure instead of plain fields and give it a reference to its owner to phone home. This would however mean that if the user changed R,G,B and A, I would convert the pixel into the bitmap's pixel format 4 times and write to memory 4 times.

  • I could return a Pixel reference from the iterator and provide it with an Update() method that needs to be called if the pixel was changed. This would be non-intuitive and risk users forgetting to call Update.

  • I could always return the Pixel by value and provide a special assignment operator. Does break the standard iterator pattern - assigning to an iterator without dereferencing should move the iterator, not update the element it is pointing at

Cygon
  • 9,444
  • 8
  • 42
  • 50
  • I think the idiomatic way is that `operartor *()` returns a (possibly const) reference to the actual pixel. Naturally the iterator could keep a reference to the original container. – dmg Feb 02 '15 at 09:47
  • Yes, but the *actual pixel* is of a varying format (for example, 16 bits per pixel with 5-6-5 bits for red-green-blue) and I would like to hide this detail from the user, thus I'm returning a proxy object, not the *actual pixel*. The iterator has a reference to the original container (`Bitmap`), of course -- my trouble lies in telling the iterator when it needs to write the changes on the proxy object back into the original container. – Cygon Feb 02 '15 at 10:01
  • Look at [Boost.Iterator](http://www.boost.org/doc/libs/1_57_0/libs/iterator/doc/index.html) and [Boost.Range](http://www.boost.org/doc/libs/1_57_0/libs/range/doc/html/index.html). – Angew is no longer proud of SO Feb 02 '15 at 10:15
  • @Cygon I see, well how about having the proxy contain a reference to the original pixel and "flush" the changes on destruction of the proxy? – dmg Feb 02 '15 at 11:27

3 Answers3

3

We've got an existing example in std::vector<bool>::iterator - that has to pull a few tricks to write to a single bit.

One solution is to return a ProxyPixel. It keeps a reference to the original pixel. You stated that updating R,G,B,A may cause 4 writes. That's true, and understandable. After the first write of just R, the underlying image should have an updated R value after all.

Or are you happy to accept an eventual update? In that case, you may delay the write-back to ProxyPixel::~ProxyPixel. Yes, the underlying image will then be temporarily out of sync as the proxy pixel is being changed, but it would be more efficient. A reasonable tradeoff.

MSalters
  • 173,980
  • 10
  • 155
  • 350
  • 2
    Eric Niebler has recently written a very informative [blog post](http://ericniebler.com/2015/01/28/to-be-or-not-to-be-an-iterator/) about the problems surrounding proxy iterators with regards to standard iterator categories (just adding this as an FYI for the OP). – Björn Pollex Feb 02 '15 at 09:58
  • Thanks @Björn, for the blog post and for mentioning *proxy iterator* - now I have a name for my problem :) – Cygon Feb 02 '15 at 10:12
  • Thanks @MSalters. I totally forgot `std::vector`. This may provide good guidance. A `ProxyPixel` that updates in its destructor is another interesting mix of advantages and concerns (even if I want to only write (eg. `std::fill()`), pixels would be read first. And sparse updates would still write to the whole bitmap while iterating over it). – Cygon Feb 02 '15 at 10:46
  • @Cygon: Don't worry too much about those redundant reads; just make the functions inlineable and let the optimizer eliminate them. – MSalters Feb 02 '15 at 14:33
0

I could put accessor methods in the Pixel structure instead of plain fields and give it a reference to its owner to phone home. This would however mean that if the user changed R,G,B and A, I would convert the pixel into the bitmap's pixel format 4 times and write to memory 4 times.

This pushes on the law of demeter (a pixel should not alter a bitmap to set it's own value); You shouldn't do this.

I could return a Pixel reference from the iterator and provide it with an Update() method that needs to be called if the pixel was changed. This would be non-intuitive and risk users forgetting to call Update.

That's a bit of a bad interface, since it will be tricky to use correctly without remembering stuff. It also violates the lay of demeter (you shouldn't update a bitmap through a pixel).

I could always return the Pixel by value and provide a special assignment operator. Does break the standard iterator pattern - assigning to an iterator without dereferencing should move the iterator, not update the element it is pointing at

Dont' do this; it violates the principle of least surprise.

Consider instead making a pixel independent from a bitmap (i.e. a pixel doesn't know what a bitmap is).

You should be able to set (valid) values into the pixel easily (with accessors for fields or an Update(R, G, B, A) or similar) and have the bitmap know what a pixel is and how to update itself from pixels, with one (or more) of these:

void Bitmap::Update(int x, int y, const Pixel& p);
void Bitmap::Fill(const Pixel& p);
void Bitmap::SetRange(SomeRangeObject r, const Pixel& p);

This way, an update operation on pixels would require the following steps:

  • get pixels from bitmap
  • update/transform the pixels as needed
  • update bitmap from pixels

This doesn't cover who will know the pixels' positions (they probably shouldn't be a part of a pixel itself), but it's a start.

With this implementation, your interdependencies stay low (functionality for updating the bitmap's pixels stays in the bitmap, not in the pixels), and the iteration model is trivial (the same as the standard iterators).

utnapistim
  • 26,809
  • 3
  • 46
  • 82
  • Indeed, that's the tried and true solution in many libraries. My motivation for the iterator (or iterator-like) design was to avoid full bounds checking and address calculation on each pixel access. -- Though unless the bitmap actually stored `Pixel` instances, I don't think the iteration model would be trivial - you'd still encounter my problem. Unless your suggestion is to convert the bitmap (or an area thereof) into a `Pixel` matrix, work on that, then write it back. – Cygon Feb 02 '15 at 13:20
  • I was thinking that you could construct pixel instances in the iterators, or in the public interface of the Bitmap. Then the Pixel instances would be a proxy/interface for editing pixels. Either way, I am not sure you can avoid the bounds checking. Have you considered implementing the bounds checking and conversion to pixel matrix as a separate (and visible) operation? Working with the bitmap then would be these steps: 1. load bitmap; 2. get pixel matrix; 3. apply pixel transformations; 4. set pixel matrix into bitmap. 5. save bitmap – utnapistim Feb 02 '15 at 13:33
  • I have to think about this :) | Extracting a pixel matrix would be viable, though I'm still resisting the idea a bit (eg. overhead on sparse updates, need to figure out area of interest before doing work). Then again, it would completely eliminate the proxy iterator issue. | Pixel as proxy is similar to my example #1 (just for clarification, in my 3 examples, `Pixel` would not know about `Bitmap`: it's a scope-limited helper class, in the most involved case it would hold a reference back to the `PixelIterator` to inform it of the appropriate time to update). – Cygon Feb 02 '15 at 14:26
  • Downvoted because several decades of experience in image processing have established that the notion of a pixel and a bitmap are so inherently intertwined that Demeter's law does not make sense. Not at all. A pixel is to a bitmap what an iterator is to a container. It would even make sense to nest the type as `Bitmap::Pixel`. – MSalters Feb 02 '15 at 14:39
  • The Law of Demeter doesn't say anything against accessing another object through a defined interface. I think @utnapistim had the impression that in my examples #1 and #2, the `PixelIterator` was supposed to deal out `Pixel` objects which then got a reference all the way back to the `Bitmap` and would modify that directly. That would have been spaghetti design and Demeter would be right to point that out :p – Cygon Feb 02 '15 at 14:47
  • @msalters - my point was simply that a pixel should exist outside of a bitmap (so you could - for example - instantiate the class without creating a bitmap first). I understand that pixels and bitmaps go together, but that doesn't mean a pixel should know it's bitmap (consider the case where you copy pixels from one bmp into another - which bitmap do they know?). – utnapistim Feb 02 '15 at 14:52
  • @utnapistim: It really doesn't make sense to copy a pixel from one bitmap to another. You can copy the `color` property from one pixel in one bitmap to another pixel in another bitmap, though. Similarly, a pixel doesn't exist outside a bitmap either. What would its {X,Y} property be? – MSalters Feb 02 '15 at 15:01
0

The members of your Pixel structure could be implemented as properties.

This would allow you to write things like 

iter->r = 0;

It would not allow you to sent the pixel struct to functions that expect it to be 'straigtforward'

Implementing a property in c++ is easy, see for example here

Writing to memory several times should not be a problem, since it will cache-local

sp2danny
  • 7,488
  • 3
  • 31
  • 53