Hi, some quick comments on the two pages making up the Collection proposal.
http://docs.codehaus.org/display/GEOTOOLS/FeatureCollection+Clean+up+Motivation About the "working with invalid data", we have to deal with it both iteration and visiting wise. So, it seems to me we should handle this at the feature collection level and have a "skip invalid features" property. FeatureCollection.setLenient/isLenient could be a way. I don't agree on splitting FeatureResults and FeatureCollection again, I would prefer to see FeatureColletion altered so that the most used methods stay there, we can try the morphing against our code and see how much it breaks. For example, keep the iterator() method, but have it return a FeatureIterator instead. Most code will stay there unchanged, but even code completion wise you would notice you're not getting back a plain Iterator. Iterator vs Visitor, I think we should keep them both because Visitor is a little alien to the average java programmer, especially when applied to something as simple as a collection, which is perceived as a sort of list, when visitor is usually applied to complex data structures that can be visited in a variety of ways. I'd say keep them both, and let the code examples speak by themselves on which one is easier and less error prone to use. http://docs.codehaus.org/display/GEOTOOLS/FeatureCollection+clean+up About visitor returning a boolean, it looks ok, maybe it could be even simpler... or maybe I'm just being overzealous. What about having a FeatureVisitor interface like: interface FeatureVisitor { void visit(Feature f); boolean keepVisiting(); } and then have the abstract visitor have a field that you can set calling "breakVisit()" within the visit method, that is: void visit(Feauture f) { if(f.getId().equals(theIdLookedFor) { feautureLookedFor = f; breakVisit(); } } The code is simpler if you don't need have "bail out in the middle" behaviour. I don't know, just a random idea. I'm very much ok with returning a boolean too. About FeatureCollection being a Feature, I still don't get it. Accessing bounds as an attribute seems silly to me, and if we need extra metadata to keep up we can add them. Most of the methods in SimpleFeature just don't apply: http://geoapi.sourceforge.net/pending/javadoc/org/opengis/feature/simple/SimpleFeature.html (and neither are the many methods I see in the superinterfaces). If someone really need to see the colleciton as a Feature, can't we have a FeatureCollection.asFeature() : Feature method that adapts it (maybe optionally implemented, so that if a collection is a mere data container it could just return null? As a final note, I'd say we should try to come up with a full FeatureCollection specification, and then submit it somehow to the users too for feedback. Also, since we're hitting this, we should try to include in this work a way to have the feature collection stored in memory, but still be usable as an input to a map, so that people can happily keep all of their features into memory, if that's what they want (but maybe we just have to fix the CachingFeatureSource I wrote some time ago for a user). Cheers Andrea ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ Geotools-devel mailing list Geotools-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel