[Insight-developers] Iterators & "operator *"

[Luis Ibanez]

Hi Gert,

Yes,
that's indeed the reason why the const_cast<> were needed.


and..
the reason why the problem didn't show up initially is
that we are missing a unit test for this particular class.


     The root of all evils:    lack of testing


Part of the reason for the absence of the test is that this
class is the base for most iterators, so it behaves a bit like
an abstract class. In either case, however, we should have had
a test for it.


We are now adding a unit test for this class.


     Luis


------------------
Gert Wollny wrote:
> Hi Luis, 
> 
> I've localised the problem,
> 
> m_Buffer is declared as
> 
> const InternalPixelType      * m_Buffer;
> 
> in itkImageConstIterator, but reused in itkImageIterator where it must
> not be const. Therefore, the const_cast<> are needed to write to values
> pointed at by m_Buffer. I'm wondering why the problem didn't show up
> when compiling the examples and/or the tests.
> 
> I'm not sure how to resolve this problem easily and make the code
> cost-correct. 
> One option would be to add a second pointer,  
>    InternalPixelType *m_WriteBufferPtr
> to the itkImageIterator. 
> Second option would be, to de-couple the two classes completely, which
> would most likely result in code duplication. 
> I'm thinking about more options. 
> 
> Best, 
> 
> Gert 
> 
> 
> 
> 
> 
>  
> 
> 
> Am Dienstag, den 27.05.2008, 11:52 -0400 schrieb Luis Ibanez:
> 
>>Hi Gert,
>>
>>We got the following secondary effects from the removal
>>of the const_cast statements in the itkImageIterator.h:
>>
>>http://www.cdash.org/CDash/viewBuildError.php?buildid=85846
>>
>>
>>Could you please take a second look at the patch and
>>suggest an alternative ?
>>
>>
>>   Thanks
>>
>>
>>     Luis
>>
>>
>>==========================================================
>>Site:marvin.jouy.inra.fr
>>
>>Build Name:Linux-gcc-Debug
>>
>>Build Time:2008-05-26 21:14:00 EDT
>>Found 4 Errors
>>
>>Warnings are here.
>>Build Log line 541
>>
>>File: Code/Common/itkImageIterator.h Line: 118 CVS/SVN
>>
>>[ 17%] Building CXX object 
>>Code/Common/CMakeFiles/ITKCommon.dir/Templates/itkImageIteratorIVUS53.o
>>/.../Insight/Code/Common/itkImageIterator.h: In member function 'void 
>>itk::ImageIterator<TImage>::Set(const typename 
>>itk::ImageConstIterator<TImage>::PixelType&) const [with TImage = 
>>itk::Templates::ImageVUS53]':
>>/.../Insight/build-debug/Code/Common/Templates/itkImageIteratorIVUS53.cxx:5: 
>>   instantiated from here
>>
>>/.../Insight/Code/Common/itkImageIterator.h:118: error: no matching 
>>function for call to 
>>'itk::DefaultPixelAccessorFunctor<itk::Image<itk::Templates::VectorUS5, 
>>3u> >::Set(const itk::Vector<short unsigned int, 5u>&, const 
>>itk::Vector<short unsigned int, 5u>&) const'
>>
>>Build Log line 545
>>
>>File: Code/Common/itkImageIterator.h Line: 125 CVS/SVN
>>
>>/.../Insight/Code/Common/itkImageIterator.h:125: error: invalid 
>>initialization of reference of type 'itk::Templates::VectorUS5&' from 
>>expression of type 'const itk::Vector<short unsigned int, 5u>'
>>
>>/.../Insight/Code/Common/itkImageIterator.h: In member function 
>>'typename itk::ImageConstIterator<TImage>::ImageType* 
>>itk::ImageIterator<TImage>::GetImage() const [with TImage = 
>>itk::Templates::ImageVUS53]':
>>/.../Insight/build-debug/Code/Common/Templates/itkImageIteratorIVUS53.cxx:5: 
>>   instantiated from here
>>
>>Build Log line 548
>>
>>File: Code/Common/itkImageIterator.h Line: 139 CVS/SVN
>>
>>/.../Insight/Code/Common/itkImageIterator.h:139: error: invalid 
>>conversion from 'const itk::Image<itk::Templates::VectorUS5, 3u>*' to 
>>'itk::Templates::ImageVUS53*'
>>
>>=============================================================================
>>
>>-----------------
>>Luis Ibanez wrote:
>>
>>>Hi Gert,
>>>
>>>
>>>   There is nothing better than a
>>>   reproducible experimental proof   :-)
>>>
>>>
>>>Thanks for reporting the bug and contributing the patch.
>>>
>>>
>>>It has now been committed to the CVS repository:
>>>http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Common/itkImageIterator.h?root=Insight&r1=1.51&r2=1.52&sortby=date 
>>>
>>>
>>>
>>>Please verify the new version at your convenience,
>>>and feel free to close the bug if you think that
>>>the issue has been solved.
>>>
>>>
>>>   Thanks
>>>
>>>
>>>       Luis
>>>
>>>
>>>
>>>---------------------
>>>Gert Wollny wrote:
>>>
>>>
>>>>Am Mittwoch, den 21.05.2008, 16:41 -0400 schrieb Luis Ibanez:
>>>>
>>>>
>>>>>Hi Gert,
>>>>>
>>>>>
>>>>>1) Yes, I do like those 'const'  :-)
>>>>
>>>>
>>>>'const' is great, no questions. Actually, the const can stay and
>>>>const_cast<> go away :-) http://www.itk.org/Bug/view.php?id=7085
>>>>
>>>>Best,
>>>>Gert
>>>>
>>>
> 
>