[Insight-developers] ImageRegionIterator consistency issues

[Zachary Pincus]

On closer inspection, it seems like the way in which image iterators  
are interconverted in ITK could use a review.

The documentation states "Many routines return an ImageIterator but  
for a particular task, you may want an ImageRegionIterator.  Rather  
than provide overloaded APIs that return different types of  
Iterators, itk returns ImageIterators and uses constructors to cast  
from an ImageIterator to a ImageRegionIterator" (and so forth for  
other iterator subclasses).

The problem is that the implementation of this is patchwork at best.

Specifically, there are multiple base classes for image iterators:  
ImageConstIterator, ImageConstIteratorWithIndex, and  
ConstNeighborhoodIterator (at least). Assuming that conversion  
between the first two and the last is uncommon, this means that there  
need to be methods to convert an ImageConstIterator to an  
ImageConstIteratorWithIndex, and vice-versa.

In fact, ImageRegionIterator has a constructor which claims to accept  
an ImageIteratorWithIndex. But of course there's no method to  
accomplish this, so that function would fail to compile if it were  
ever used.

Why this constructor is defined in ImageRegionIterator and nowhere  
else, and why it takes an ImageIteratorWithIndex and not the  
ConstIterator version, and so forth, are of course completely  
mysterious.

Perhaps defining a more strict policy about how iterators need to be  
converted would be best? Otherwise perhaps the whole bit ought to be  
deprecated (given that much of this API fails even to compile, I  
doubt anyone's using it), and people who need to convert between  
iterator types will have to do so manually, depending on their  
specific need?

Otherwise the policy would look something like this:
(1) Iterator base classes will implement constructor methods to  
accept iterators of other base class types.

(2) Other iterator classes will implement constructors which accept  
each iterator base class type (which call the superclass constructor  
methods, plus do any additional calculations necessary). Doing things  
via calling superclass operator= methods, as done currently, seems  
marginally suspect.

This will ensure that basic iterator conversion works. There could be  
other constructors defined to allow more precise conversions (e.g.  
ImageRegionConstIterator <-> ImageRegionConstIteratorWithIndex  
without losing the region information that would happen if the  
conversion went through the non-region-enabled superclasses.)

But this all seems like more work than may be worth while, given the  
fact that this API is partially unused (and partially untested, and  
partially  doesn't compile).

Zach


On Jan 26, 2006, at 2:02 AM, Zachary Pincus wrote:

> Hello again -
>
> Today Gaetan and I have uncovered some minor consistency issues in  
> the ImageRegionIterators. One issue I can fix (and will in the next  
> few days); one I am not sure of what to do about.
>
> --------
> Issue the first:
>
> itkImageRegionConstIterator.h defines a constructor that looks like  
> this:
>   ImageRegionConstIterator( const ImageIterator<TImage> &it)
>   {
>     this->ImageIterator<TImage>::operator=(it);
>     ...
>   }
>
> The only problem is that ImageRegionConstIterator doesn't inherit  
> from ImageIterator, so the first line of the constructor doesn't  
> compile. Since this iterator inherits from ImageConstIterator, the  
> first line should look like this:
>     this->ImageConstIterator<TImage>::operator=(it);
>
> This should fix things.
>
> --------
> Issue the second:
>
> itkImageRegionIterator.txx defines a constructor that looks like this:
>
> template< typename TImage >
> ImageRegionIterator<TImage>
> ::ImageRegionIterator( const ImageIteratorWithIndex<TImage> &it):
>   ImageRegionConstIterator<TImage>(it)
> {
> }
>
> Unfortunately, ImageRegionConstIterator doesn't define any  
> constructors that are compatible with an ImageIteratorWithIndex  
> parameter, so this method also fails to compile. I'm not sure what  
> the correct approach would be for fixing this issue.
>
> Zach
>
> _______________________________________________
> Insight-developers mailing list
> Insight-developers at itk.org
> http://www.itk.org/mailman/listinfo/insight-developers