Andrea Aime wrote: > so yesterday I was looking into how making validation > optional into the current feature model. > yeah! > First off, we have this idea that features might be > allowed to hold invalid attributes, and that we can > use a isValid() method to check whether they > are actually valid or not. > understood. > Implementation wise, this triggers the first issue: > that method does not tell what is wrong, but only > that something is wrong. > A different approach could be to have a > validate() throws IllegalAttributeException > and have the exception report exactly what's wrong > with the feature. > Difficulty is that this either needs to be a structures exception message (indicating say the xpath to the problem and kind of problem) or we should provides a utility method to do this kind of check. > Moreover, even having that method, someone might want to have continuous > checks on the feature model > like we do now, that is, have the builders and the model throw exceptions > when an invalid > attribute value is provided. > > Do we really want to do that? Or we just tell the world that validation > checks must be explicit, that is, never ask the model to do them > automatically? > I had pictured a useful compromise; - have validation off by default - have always-on-validation as a "strict" option to help debug problems
Given the choice; validation off is the way to go. > If on the contrary we believe it's important and we allow the user to > customize that behaviour, Jody was suggesting to do so by providing two > different feature factories, > one that builds non validating features, the other that builds > validating ones... which in the end would boil down to > parametrize the SimpleFeatureImpl with a flag that enables > validation when attributes are set. > It would also require the builder static utility methods, which > are very commonly used in the datastores, to accept a factory. > To make it effective it would mean we'd need to add a new > Query hint (the feature factory, which is not there) and use it > in all data stores. > Yet that would not be all, those methods are used in > many many places (SimpleFeatureBuilder.build(...) is used in 179 > different places according to Eclipse), do we want add a factory > param in each place? Just have a look in DataUtilities class > to get an idea. > It was these bigger picture questions that were being talked about as the builder was introduced. The SimpleFeatureBuilder.build method is always a short cut hack; mostly to enable the DataUtility methods to work ... How about for these cases we consider "validation off" to be the rule rather than the exception. > Given that most non datastore methods are transformation related ones, we > could use the same factory that built the original features, adding a > getFactory() method to the Property interface... > thought of course that would be another geoapi change, would > require even more changes around, well, you get the picture. > > It seems to me this kind of change would require at least a one > day sprint... what do we do in the meantime? > Backing up: - Our goal was to make the FeatureTypes static utility method available for people to find; we have now done that correct? - For detailed reporting on why something is invalid - a good idea - we should treat that as a separate problem; it is something people can use a utility method for. Once we understand the problem we can consider adding it to the API. Code first then API. - We should only worry about the use of SimpleFeatureBuilder as a builder ie the static methods on SimpleFeatureBuilder and in DataUtilities are assumptions used throughout our library; they saved us time when making the migration. You should be able to inline SimpleFeatureBuilder.build() as needed... - For the static methods we should make use of a system wide FetaureFactory - assuming validation off by default. Aside: The idea of remembering the factory seems like a little bit of gain; while I had similar thoughts they come from a different problem When working with Shapefiles only a limited set of content is supported (max string length etc...). Having ShapefileDataStore.getFeatureTypeFactory() return a factory that knows about these limitations is a sensible way to go... Jody ------------------------------------------------------------------------- Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW! Studies have shown that voting for your favorite open source project, along with a healthy diet, reduces your potential for chronic lameness and boredom. Vote Now at http://www.sourceforge.net/community/cca08 _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
