Vitali Diatchkov wrote:

org.geotools.feature.collection.DelegateFeatureIterator delegates actions to
java.util.Iterator that is actually org.geotools.data.store.
FeatureReaderIterator.
Okay that makes sense .... with you so far.

DelegateFeatureIterator must close internal iterator, but no ways to cast it
to FeatureReaderIterator because of "package" visibility of the last one.
DelegateFeatureIterator was supposed to only be used when the contents were backed by a *real* java Collection. It may check the collection to see if it is a ResourceCollection .... I will check the code
this evening and get back to you.

Because of this at least four places in UDIG  lead to unclosed shapefile
readers and locking in READ mode forever.
Opps!

What's the real need of delegation of iteration.. It is not enough to close
iterator internally inside of hasNext() method as it is in
FeatureReaderIterator because  sometimes only first feature is needed to be
read and internally it is not closed automatically inside of hasNext() while
more features are there, but implicitly it is impossible to close
DelegateFeatureIterator because of empty close() method..
Understood we do have a solution for this one. We are supposed to use FeatureCollection.close( iterator ) - which
has the correct package visiability to perform the close opperations.

I'm confused with that mess of different iterators, etc..
We started out with FeatureReader & FeatureWriter - basically Iterators with IOExceptions. We are transitioning towards something more normal - ie. Basic Java Iterators. But like Hibernate we need to be able to support a "close" opperation. We stole their solution of
putting the close method on the collection class that produce the iterator.

Solution: Let's make org.geotools.data.store.FeatureReaderIterator as a
public and inside of org.geotools.feature.collection.DelegateFeatureIterator
the method close() looks like:

        public void close() {
                if(delegate != null){
                        
                        if(delegate instanceof FeatureReaderIterator)
                                ((FeatureReaderIterator)delegate).close();

                }
                delegate = null;
                
        }
See I would rather the DelegateFeatureIterator knew the collection where its delegate came from and
could perform an instance of check:

public void close() {
  if( delegate != null && collection != null && collection instanceof 
ResourceCollection ){
    ((ResourceCollection)collection).close( delegate );
  }
  delegate == null; collection == null;
}

Or refactor DelegateFeatureIterator into org.geotools.data.store package
while keeping FeatureReaderIterator with "package" visibility..
I would much rather have all the "book keeping" classes needed when Implementing a DataStore in org.geotools.data.store, especially with respect to the JDBCDataStores we need to smack these implementations into a maintainable sense of order.

Isn't it preferable to decrease the number of different iterators, etc..?
See above - it is possible to reduce the API but I am not going to suggest it until GeoTools 3. They are however getting steadly
depreciated...  In terms of different implementations my preference is:
a) Datastore writer does something efficient for all of them them (as an example Jesse got Shapefile to go 20x faster by making an iterator based on the index) b) Provide helper methods that lets the datastore writer implement one technique, and then adapt their efficient implementation to the other needed APIs.

FeatureReader is actually a good low-level abstraction - I am sorry it leaked out into user space :-( I was not assertive enough at the time (being new to geotools at the time).

I would love to know where in uDig FeatureReaders were used, and see what we can do about it.

Jody



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
Geotools-devel mailing list
Geotools-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to