Patch applied. Shall we open a new issue for breaking simple feature type out of the complex type class hierarchy?
Justin Deoliveira wrote: > Ok, I propose applying Ben's patch for the moment since simple feature > type is pretty clearly broken. And make the clean break from the super > class a separate issue. > > Jody Garnett wrote: >> Hi Ben sorry to be slow to respond to this one; I am busy prepping for a >> meeting with Simone. I do have your patch applied but have not been able >> to review it carefully yet. >> >> Justin you are correct about the duplication being an issue; I am still >> keen to make a SimpleFeatureType / SimpleFeature implementation that is >> lean and does not extend another class. >> >> Jody >> >> On Fri, Feb 27, 2009 at 5:05 PM, Justin Deoliveira <[email protected] >> <mailto:[email protected]>> wrote: >> >> Hi Ben, >> >> >> Ben Caradoc-Davies wrote: >> >> Justin, I have no objection. It should do no harm. However, note >> that: >> >> (1) order should never be important, except for special case >> subclasses such as SimpleFeatureTypeImpl (see below), and >> >> I guess the contract does not specify any order... but it just seems >> like a good thing to do implementation wise. Unless there is a >> strong case for changing the order... since types are immutable i >> can't think of one. >> >> >> (2) LinkedHashMap conforms to the Map contract, which guarantees >> that order is *not* significant for equals/hashCode. Fixing >> iteration order does not fix this. >> >> Good point, something i missed when reading the javadoc of >> LinkedHashMap. >> >> >> I experimented with using LinkedHashMap in ComplexTypeImpl to >> fix SimpleFeatureTypeImpl iteration order, but when I realised >> it did not fix equals/hashCode, I made all my changes to >> SimpleFeatureTypeImpl. If you support my position, please join >> me in nagging Jody to get my patch accepted. Please take a look >> at it; the patch comes with unit tests for iteration order >> consistency: >> http://jira.codehaus.org/browse/GEOT-2338 >> >> More importantly, why do you want to change the order of >> properties? Do you have a non-simple use case in which they >> matter? If not, please consider my patch. >> >> Indeed this is the exact case addressed by the patch, calling >> getDescriptors() and it returning descriptors in a different order. >> So I am +1 on fixing this at the simple feature type level if the >> current behavior of complex feature type is deemed necessary. The >> patch looks good however the duplication of storage of descriptors >> for simple feature type seems wrong... perhaps SimpleFetaureTypeImpl >> should not extend ComplexFeatureTypeImpl. >> >> >> Kind regards, >> Ben. >> >> >> Justin Deoliveira wrote: >> >> Sorry, i did not mean tree map, just a map with predictable >> iteration order. LinkedHashMap should do the trick. >> >> Justin Deoliveira wrote: >> >> Hi all, >> >> I think Ben may have brought this up recently, but >> looking at ComplexTypeImpl it seems property values are >> stored in a hash map, which totally throws away the >> order in which property descriptors are added to it. >> >> Any objection to making this a tree map as to maintain >> order? >> >> >> >> >> >> -- >> Justin Deoliveira >> OpenGeo - http://opengeo.org >> Enterprise support for open source geospatial. >> >> > > -- Justin Deoliveira OpenGeo - http://opengeo.org Enterprise support for open source geospatial. ------------------------------------------------------------------------------ Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise -Strategies to boost innovation and cut costs with open source participation -Receive a $600 discount off the registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
