[Insight-developers] itkSubsample issues

[Brad Davis]

I propose changing the implementation of GetMeasurementVector in the
Subsample class from

const MeasurementVectorType & GetMeasurementVector(const
InstanceIdentifier &id) const
{ return m_Sample->GetMeasurementVector(id) ; }

to

const MeasurementVectorType & GetMeasurementVector(const
InstanceIdentifier &id) const
{ return m_Sample->GetMeasurementVector(m_IdHolder[id]) ; }

First, the current implementation allows access to measurement vectors
that are not in the subsample---GetMeasurementVector gets vectors from
the sample, not the subsample.  Second, while the
GetMeasurementVectorByIndex() method can be used to return the
measurement vectors that are actually in the sample, the change in
interface means that it is difficult to write general code that will
work for a sample or a subsample.  Also, GetMeasurementVectorByIndex
returs by value rather than by reference.  Iterators can be used but
using both iterators and GetMeasurementVectorByIndex() it is
impossible to have a working subsample of a subsample---the
implementation only allows for one level of indirection.  Again this
means that you have to know a priori if you are working with a sample
or subsample.

The change that I propose allows Samples and Subsamples to be
addressed by a common interface, and allows the programmer to create a
Subsample of either a Sample or a Subsample.  This would increase the
flexibility of the classes and allow for more general code.

The drawback of the change is that if someone is already using the
GetMeasurementVector() method of the Subsample class to directly (and
incorrectly, I argue) access vectors from the Sample, then the change
is not backward compatible.  However, access to the Sample can also be
gained by Subsample's GetSample method.

The other mechanism for fixing this problem is to add a
GetMeasurementVectorByIndex() method to the Sample class, and change
the method to return by reference.  This does not require changing the
GetMeasurementVector method but it makes the classes more confusing.

Brad