Hi Adrian, Adrian Custer wrote: > Hey Justin, > > Your builders look really nice; seems like they will be fun to use.
Thanks :) > > > Martin and I discussed a bit the role of the CRS in the feature type. > This issue will only last until geotools moves to using DirectPositions > rather than JTS directly, however the issue is with us today. > > We suggest that adding a jts geometry to any featureType whose crs has > not been explicitly set through some call to .setCRS(..) or to .crs(..) > should generate a WARNING. > Agreed. > We suggest therefore that a user be allowed to set a CRS to null, e.g. > via a call .setCRS(null). That would allow the user to use geometries > for which they do not know the CRS but force them to set that > explicitly. Once the CRS is set to null (or to any other value), no > WARNING would be emitted. Ok, let me make sure I am getting this right. So the following code should generate a warning: SimpleFeatureTypeBuilder b = new ...(); b.setName( "foo" ); b.add( "geometry", Point.class ); SimpleFeatureType type = b.buildFeatureType(); But the following code should not: SimpleFeatureTypeBuilder b = new ...(); b.setName( "bar" ); b.setCRS( null ); b.add( "geometry", Point.class ); SimpleFeatureType type = b.buildFeatureType(); > > Does that work for you? Perhaps this is already the case. > The way the builder is written now it is ok for the crs to be set to null, and there is no default. > > Note also I changed the Simple example your page: > http://docs.codehaus.org/display/GEOTDOC/Feature+Model+Guide > to set the CRS before adding the geometry because I figured we wanted to > encourage 'good practice' in the doc. We should add, in one of the > alternative sections, .setCRS(null) if and when such functionality > exists. Sure that works for me. Looking at that page I do have one issue with one of the code examples. Its the the "Multiple CRS for Geometries". The setCRS(..) method is not meant to be called twice. It is setting "global" state so it should really be called once. When setting "per attribute state" the crs(..) method should be used. For instance, until recently feature types themselves had a crs. This has been removed now but the setCRS would map to that crs, the one of the feature type itself. And actually in implementation one thing we might actually want to do is set the crs as user data. By using setCRS to set the global context for each attribute that is added you are clobbering it each time. Is that making sense? I also think that in the first example, the "chaining" alternative should be removed. The intent of chaining is to provide a nice way to differentiate between global and per attribute state, not to make the code sample less lines of code. So I suggest we remove it as its just one less thing for the user to see, and stick to using chaining where there is an actual need for it. Also... keep on thing in mind. That document is not intended for users at this time. It is intended for other geotools developers at the code sprint. So I am not worried about getting it absolutely right at this time. > > --adrian > > > ------------------------------------------------------------------------- > This SF.net email is sponsored by: Microsoft > Defy all challenges. Microsoft(R) Visual Studio 2005. > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ > _______________________________________________ > Geotools-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/geotools-devel > > !DSPAM:4007,46f2444722133668746562! > -- Justin Deoliveira The Open Planning Project http://topp.openplans.org ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
