On Tue, Sep 4, 2012 at 4:21 AM, <[email protected]> wrote:

> Hi Andrea,****
>
> ** **
>
> Thanks for your feedback and sorry about the formatting issues – I was
> running the formatter over the whole document as I thought they were meant
> to comply with the guidelines but Ben has explained to me that any existing
> code’s style should take precedence.****
>
> ** **
>
> I’ve fixed the formatting, rolled back changes on ContentFeatureSource and
> removed the deprecation annotation from AbstractDataStoreFactory.
>

Hey, sorry for the late feedback, I'm presently on a deathmarch to build
three large features in GS/GWC


> ****
>
> ** **
>
> I was looking at H2DataStoreFactoryTest as it has an external dependency
> on a specific port that was in use on my Windows machine, causing my Maven
> build to fail. Have reversed changes so this can be dealt with as a
> separate issue as advised.****
>
> ** **
>
> As for the ContentComplexFeatureSource and its unimplemented methods: my
> intent here was to create this class as a complex analogue of the
> simple-feature-exclusive ContentFeatureSource, in order to make the API as
> consistent as possible between the acquisition of both simple and complex
> features... having said that, I don’t really know exactly what should be in
> these implementations and how they ought to differ from their simple
> counterparts. I’d really like to get some feedback on my work to date to
> make sure that I’m on the right track with everything. I have a sample code
> (#1) that demonstrates utility of these changes by parsing a WFS response
> into complex features... if this part is OK then I guess the
> ContentComplexFeatureSource’s implementation can be fleshed out as need be.
> ****
>
> ** **
>
> #1:
> http://docs.codehaus.org/display/GEOTOOLS/ComplexFeature+Parsing+and+Building+Support
> ****
>
> ** **
>
> Thanks again for your help and advice,
>

Looking again at the patch, I still see ContentComplexFeatureSource
implementing many compulsory
method with  throw new RuntimeException("Not implemented");, that is
clearly unacceptable,
if a method has to be implemented, it's not optional, you have to make it
abstract so
that the implementor is forced to implement it.

The same goes for WFSContentComplexFeatureCollection, which implements key
methods
such as feature iterator closing (the methods that frees the resources held
by
an iterator) as throw new UnsupportedOperationException();, that is also
unacceptable,
only the modification methods in a FeatureCollection are actually optional
and can
be implemented that way.

The work of a base class is to provide all of the generic "meat" that a
developer
not looking for particular performance features is not forced to implement,
leaving the developer to concentrate on the very basics, like iterating the
features.
ContentFeatureSource does an excellent job at that, allowing the developer
to start
easy and get with little effort a fully functioning feature source,
and gradually grow towards something that is production worthy, adding
native
filtering, sorting, retyping and the like.

Having a base class that instead implements compulsory methods by throwing
unsupported operation exceptions is worse than not having it at all, at
least
when implementing the interface directly you're forced in thinking about
what
is really needed.

I see a lot of changes in app-schema land, haven't looked at them, but I
hope
Ben did, and in the WFS data store, which I did not look at either, and
really
hope Gabriel will have a look at

Btw, thanks a bunch for the builders, I've used them in GeoServer CSW work
to build CSW Record objects (which are complex features), btw, I did
make some modifications on them to make them handle a few cases that were
missing, you have have a look at their current state here:
https://github.com/geoserver/geoserver/tree/master/src/community/csw/csw-api/src/main/java/org/geotools/feature

Cheers
Andrea


-- 
==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more
information.
==

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax:   +39 0584 962313
mob:   +39  339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://ad.doubleclick.net/clk;258768047;13503038;j?
http://info.appdynamics.com/FreeJavaPerformanceDownload.html
_______________________________________________
GeoTools-Devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to