Thanks for the feedback Gabriel. Some comments inline.

Gabriel Roldán wrote:
> On Thursday 07 February 2008 01:18:35 am Justin Deoliveira wrote:
>> I have been giving a bit of rough feedback to others for their proposals
>> so here is a chance for people to get back at me. 
> no need to, I don't see this as personal ;)
> 
>> I finally got around 
>> to writing up my ideas on how to cleanup data access with regard to
>> feature collection.
>>
>> http://docs.codehaus.org/display/GEOTOOLS/DataStore+Cleanup
> 
> ok, the idea is sound and imho indisputable (the Implementation Complexity 
> diagram speaks by itself).
> So just some cosmetic/collaboration feedback:
> 
> - typo: "ocus the DataStore/FeatureSource api around FeatureWriter and 
> FeatureWriter" should be FeatureReader and FeatureWriter.
> 
> - wonder how many new classes we get over how many we get rid of. Looks like 
> we get rid of 18 and stick to 25 iterator+readers, plus the per datastore 
> specific (reader) ones. How many of those 25 classes are new?
The plan in my mind is to get rid of every Iterator and FeatureIterator 
decorator. This brings us down to 6 decorators (i am sure there are a 
couple i am missing). Then we implement FeatureCollection once. In that 
class we have adapters for FeatureReader to Iterator and 
FeatureIterator. So i believe it looks like this:

18 decorators + 6 adapters => 6 decorators + 2 adapters.

And this does not take into account any of the datastore specific readers.
> 
> - wonder how would they play with plain Feature. What happens if they're 
> phrased in term of features or parametrized?
I guess this plays off the other proposal that is going on. Depending on 
the hybrid subclass / parameterization we come up with.
> 
> - naming: DataStore2 and ContentDataStore do not communicate their intents 
> well. Or at all.
> 
Agreed, ContentDataStore was somethign that someone else named and was 
experimental. Ideally we add these methods directly to datastore. I only 
used DataStore2 to stress that they are additions. But good point, that 
needs to be made clear in the proposal.
> - seems like this proposal and the feature access one should work together in 
> order to ensure a one time revamp with as few glitches as possible.
> 
Agreed.
> - personally would like to get rid of the getXXX(Filter) methods and stick to 
> just getXXX(Query)
I added the Filter equivalents in order to be consistent with the rest 
of our api. For instance FeatureSource has getFeatures(Query) and 
getFeatures(Filter), so i stuck with the same pattern. But i could go 
either way. Perhaps i will take them out of the proposal since they are 
just convenience at this point, and we can add them later if necessary.
> 
> - getWriter(): being both an inserting and updating writter looks 
> anti-natural. Not sure the exact use case it serves though, but you already 
> have getWriterInsert(), good, and getWriterUpdate(), good.
Yeah... i dont like the dual mode writer either. But this is how 
DataStore.getFeatureWriter() works currently so i thought it would be 
good to have it as well. What do others think on this one?
> 
> 
> good stuff,
> 
> Gabriel
>> -Justin
> 
> 
> 
> !DSPAM:4007,47aa68ff265127180515871!
> 


-- 
Justin Deoliveira
The Open Planning Project
[EMAIL PROTECTED]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to