id,summary,reporter,owner,description,type,status,priority,milestone,component,version,resolution,keywords,cc,state
590,geometry refactoring,tschaub,tschaub,"There are a number of issues that lead me to a little refactoring of the geometry code.  These are mostly tickets #589, #588, #587, #584, #583, and #582.  I know this won't be the easiest patch to review, but it is largely patches to the tests.

Additional justification for items (in the order they appear in the patch):

1) All collections get a componentTypes array.  This is an array of strings.  If null, then any type of geometry can be added to the collection.  Otherwise, the class name of a geometry must appear in the array if it is to be added to the collection.  This allows the addComponent method to be removed from the collection subclasses (with the exception of LinearRing) - as that method was largely used to check the component type before adding it.

2) Modifying the clone method for collections allows subclasses to inherit this method directly.  A subclass of collection creates a clone based on its own class name (makes #584 a duplicate).

3) In the old collection.addComponent code, if the index was 0, it would be ignored - this is wrong (takes care of #588).  In addition, I'm using the return of addComponent to indicate whether the component was added or not (#587).

4) Collection needs an equals method.  This is inherited by all subclasses (takes care of #583).

5) Curve gets a contentTypes array and loses its clone method.

6) LinearRing gets a contentTypes array, some docs, and loses its clone method.

7) LinearRings are special (#589).  In the spirit of being liberal with what we accept, a linear ring can be constructed with an array of points that is open or closed.  If it is open, the resulting linear ring will be closed.  If it is closed, the resulting linear ring will be closed.  If you wish to do something crazy and override this behavior, you can call addComponent with an index as the second argument.  In this case, the method doesn't check if you are adding a duplicate point.  This method was also relying on the LineString and Curve prototypes - it only needs the Collection prototype here.

8) Removing points from linear rings was also relying on the Curve and LineString prototypes.  Collection.prototype does the trick here.

9) LineString inherits its componentTypes from Curve and loses its clone method.  Also, removeComponent should use the method from Collection - the extra lookup is not necessary as Curve doesn't have its own.

10) MultiLineString gets a componentTypes and loses its addComponent method.  Same for MultiPoint, MultiPolygon, and Polygon.

11) Handler.Path uses the new behavior of addComponent to override the no-duplicate point rule for LinearRing.  We want to add duplicate points here because they immediately start moving to a new position.

12) Handler.Polygon gets its own dblclick method because it needs to remove its penultimate point instead of its ultimate point (this makes #582 a duplicate).

13) The rest is tests to prove it all works.

I know this should be in many small patches, but I didn't encounter all this in a linear fashion - and the patch, review, commit cycle didn't seem appropriate for each piece.

",bug,closed,minor,2.4 Release,Geometry,,fixed,,,