[Geotools-devel] SDO Geometry Hole (aka GEOT-239) Tag

[Jody G. <jga...@re...>]

We have been playing tag on:
http://jira.codehaus.org/browse/GEOT-239

Were we got to was basically this:

> After reading the code of SDO.java again, it looks that only the test 
> to see if the input conditions are ok is wrong.
> It was
>
> if (!(STARTING_OFFSET >= 1) || !(STARTING_OFFSET <= coords.size()))
> throw new IllegalArgumentException("ELEM_INFO STARTING_OFFSET 
> "+STARTING_OFFSET+" inconsistent with ORDINATES length "+coords.size());
>
> I changed it to
> if (!(STARTING_OFFSET >= 1) || !(STARTING_OFFSET <= (coords.size() * 
> D(GTYPE))))
> throw new IllegalArgumentException("ELEM_INFO STARTING_OFFSET 
> "+STARTING_OFFSET+" inconsistent with COORDINATES length 
> "+coords.size());
>
> The rest of the source file seems to handle the STARTING_OFFSET 
> correctly so we had an exception while all was OK ...

I think you have found a bug - and one that is common everywhere. This 
bug will only show up when we have multiple geometry that are encoded in 
the same ordinate
array (so Polygon and the Geometry Collections).

I have factored out the size calculation:
 ordinateSize( coords, GTYPE )

And will switch the test to:
if( !(1 <= STARTING_OFFSET && STARTING_OFFSET <= ordinateSize( coords, 
GTYPE ))){
      throw new IllegalArgumentException(
                    "ELEM_INFO STARTING_OFFSET "+STARTING_OFFSET+
                    "inconsistent with COORDINATES length 
"+ordinateSize( coords, GTYPE ) );
        }
On second thought we need to do a lot of this - creating an ensure 
method that throws IllegalArgumentExceptions
allows us to phrase our tests in the positive sense:
        ensure( "ELEM_INFO STARTING_OFFSET {1} must be in the range 
{0}..{1} of COORDINATES",
                1,STARTING_OFFSET, ordinateSize( coords, GTYPE ) );       
        ensure( "ETYPE {0} must be expected POLYGON or POLYGON_EXTERIOR 
(one of {1})",
                eTYPE, new int[]{ ETYPE.POLYGON, ETYPE.POLYGON_EXTERIOR } );