[ImageJ-devel] proposed changes in the ImgLib2 abstract class hierarchy

[Stephan Preibisch]

I agree, makes sense!

On Mar 13, 2012, at 17:00 , Lee Kamentsky wrote:

> That's a lot nicer. It makes it much easier to pull the abstract implementations into everything.
> 
> On 3/13/2012 4:01 PM, Tobias Pietzsch wrote:
>> Hi guys,
>> 
>> while I was writing ImgLib docs, I noticed that the current
>> abstract class hierarchy which leads to AbstractRandomAccess<T>
>> is a little bit stupid (see attached imglib2-abstract-current.png)
>> 
>> There is no need for the abstract class hierarchy to pull the
>> Sampler<T> interface all the way through.  The Sampler<T> is only
>> implemented in the concrete classes.  There is really no need to have
>> the dependency on T in the abstract hierarchy.  This prevents for
>> instance RealPoint to make use of the RealPositionable and
>> RealLocalizable implementations in AbstractRealRandomAccess<T>.
>> 
>> Therefore I propose to change the hierarchy as shown in the
>> second attached diagram, imglib2-abstract-proposed.png.
>> 
>> I implemented those changes for the "real" path, see branch
>> "modified-abstract-hierarchy".  Note how nicely RealPoint fits
>> into the picture now :-)
>> Before I go ahead and do the same for the integer accessors, I wanted
>> to ask whether you agree that this is a good thing to do...
>> 
>> There is one more thing: The AbstractRealRandomAccess implemented
>> the complete RealPositionable interface, while AbstractRandomAccess
>> only implements part of Positionable - with the implemented part
>> based on the unimplemented methods.  The idea was that if a derived
>> concrete accessor compiles you know you implemented what was necessary.
>> 
>> However, I found the approach of AbstractRealRandomAccess to implement
>> anything quite nice when using it. I would suggest doing the same for
>> integer.  Then either
>> * you know what you are doing when overriding (parts of) the
>>  Positionable implementation
>> * you leave it as is (e.g. Point)
>> * you derive from AbstractLocalizable and implement Positionable
>>  completely on your own.
>> 
>> Thoughts?
>> 
>> best regards,
>> Tobias
>