Page MenuHomePhabricator

PointSetDifferenceStatisticsCalculator iterates over wrong IDs
Closed, ResolvedPublic

Description

Our pretty useful class PointSetDifferenceStatisticsCalculator internally iterates over two given input PointSets via an integer which is set by PointSet->GetSize() and just incremented.

This works nicely, if the IDs of the input PointSets (Size n=3) are like this:
0,1,2

If not every integer is assigned to a valid ID, like this:
7, 12, 17

the method will use [0, 0, 0] points for each non-existing ID and thus give very nice result (difference between PointSets = 0).

This bug is very critical, as it is potentially hidden. I just run into it, because my PointSets had the size n=400 and my first ID was 7000. I could not believe that the difference between my two PointSets should be 0 for every value.

A simple fix is to iterate over the PointSets with a correct iterator:
mitk::PointSet::PointsIterator pointSetIterator = m_PointSet1->Begin();
mitk::PointSet::PointsIterator pointSetIterator2 = m_PointSet2->Begin();
mitk::PointSet::PointsIterator end = m_PointSet1->End();
while(pointSetIterator != end)

{
  point1 = pointSetIterator.Value();
  point2 = pointSetIterator2.Value();

[...]

I will fix this bug and enhance the test, but we should do some manual testing for the next release.

Event Timeline

[6d541d]: Merge branch 'bug-15620-PointSetDiffCalcIterator'

Merged commits:

2013-08-05 16:35:16 Thomas Kilgus [e8af29]
Changed while loop to for loop iterating over both sets.


2013-08-05 16:28:04 Thomas Kilgus [ccfac0]
Rewrote test to have a nice example for testing. Added a test case for the bug.


2013-08-05 15:11:14 Thomas Kilgus [2a1020]
Using iterators to iterate over points now in order to prevent reading from point IDs which do not exist.