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 <jdeol...@opengeo.org>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.
>
------------------------------------------------------------------------------
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
Geotools-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel
