Justin Deoliveira wrote: > Great review Andrea. Here are my comments. > Looking forward to looking at your review Andrea :-) > * getUserData()/putUserData() > This is a big plus for getting things done (rather than forcing information into some stupid metadata class or format). The real thing we want to prevent is application writers setting up a bunch of Maps<FeatureType,Object> caches - and then trying to keep them in sync.
> * Java Bean Conventions > > +1, and they are probably inconsistent in some places. We can still > model convenience methods with he absence of a setter method. > +1 > * CRS > > Agreed. And my opinion is that too many things have CRS's on them. > Currently you have the following methods. > > - FeatureType.getCRS() > - Feature.getCRS() > - GeometryAttributeType.getCRS() > - GeometryAttribute.getCRS() > > Its confusing... and undocumented how each relates to each other. I have > seen the results of this in starting to write code... and trying to > figure out the crs is a 4 level deep nested if-else statement. > How do you guys want to handle this? As I understand it an srsname can appear in any of those locations in GML. You could remove it from FetureType and GeoetryAttributeType and express it as a constraint on the valid values (but that would kill usability). I think the place we *want* it for readable code is in in GeometryAttribute - so we can quickly figure out the crs for a geometry without guess work. And also in GeometryAttributeType - so we can quickly figure out what projection any content we are creating needs to be in. For Feature and FeatureType perhaps we can leave that information up to the GML parser (to be context sensitive) and it can use putClientProperties if it really cares to store the context on the FeatureType. > * InternationlizedString > > Can we kill this? I don't see the point of modeling i18n directly in the > api since every application does i18n differently. It would be one thing > if this was a subclass of String, but its not. I know it extends > CharSequence but still... I dont really see any other java api's that do > this. > Where is this being used in the feature model? I find that InternationalString is great, and really lets us have a smooth mapping from ISO standards to Java. The break down of human readable vs machine readable text is explicit and leaves no guess work. If you can give me specific things to look at I will check the ISO19107 and see if it actually supposed to be human readable. > * FeatureCollection > > I think we should kill this all together from geoapi. It adds a ton of > complexity (associations and stuff) with very little gain. Especially > since we recently decided how to model feature collection in geotools > and it involves not making FeatureCollection a feature. > I would like some feedback on this, I don't mind stripping it out but it does seem to be a break with GML and the OGC reference model. There is nothing special about FeatureCollection; the same result can be had by making a Feature that has an association called "members" (I just find it useful to have this common case captured explicitly so we can provide an iterator / visitor to navigate the contents. GeoAPI is not complete with out a representation of FeatureCollection, if we want to make it not implement Feature then we should do that. If you guys want to remove the geoapi FeatureStore class then you can remove FeatureCollection as a concern for later; but until then we need something. > * Choice/Sequence > > These appear to be unused. Hopefully Gabriel can better comment on the > usage of these. > I am not sure if they are needed? Creating your ComplexType with a List<AttributeDescriptor> is good enough to indicate sequence. Choice is represented with a rather stupid filter constraint; but there is nothing special about the interface. They do service as a good documentation on "how to do it"; but perhaps that is where they should be kept? Jody ------------------------------------------------------------------------- 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
