- Whats Up?
- Monthly Release
- Query Hints
- PrePostFilterVisitorSplitter Confusion
- Factory Finder=
- FeatureCollection
<jdeolive> does anyone have any agenda items
<jgarnett> 0) what is up
<jgarnett> 1) monthly release
<aaime> 2) Query hints (again)
<sfarber> 3) PrePostFilterVisitorSplitter confusion (GeoAPI filters, etc.)
<jonathanv> don't forget featurecollection
<gdavis> 4). factory finder stuff is not working as martin suggests
<jgarnett> gdavis - geometry ready to be supported yet? Or are you just stuck behind factory finder stuff?
<aaime> 5) featurecollection (happy? 
)
<jdeolive> sorry, jonathonv, did you want a specific item for featurecollection?
<jdeolive> ok
<jonathanv> no
<gdavis> im stuck behind factory finder stuff before I can get back on supported status stuff. it's basically ready once the factory finder stuff works
<jdeolive> anyone else?
<jdeolive> looks like a full one so lets get started, if anyone has anything else just add it on
<jdeolive> 0) whats up
<jdeolive> everyone can sound off here
<jdeolive> jdeolive: geoserver 1.5.2 release
<aaime> me too
<aaime> added fast oracle bbox computation today (it was too silly it did not have one...)
<jgarnett> jody: factory finder is not my friend, having frun reviewing the FM work on my own time
<gdavis> im trying to make a factory finder for the unsupported geom module, so that i can get it supported. the factory finder stuff doesn't appear to be working tho
<gdavis> anyone else?
<jdeolive> ok, lets move on
<jdeolive> 1) monthly release
<jdeolive> it is still on my list fo release 2.4-RC0 and 2.5-M1
<jdeolive> i was going to try on the weekend... but didn't get around to it 
<jdeolive> although i still want to run geoserver cite before 2.5-M1
<jdeolive> but i think 2.4-RC0 is priority anyways
<jdeolive> so i am going to try and release it this week after the geoserver release
<jdeolive> anyone have any objections?
<jgarnett> We are still on to release 2.5-M0 this week
<jgarnett> (normal monthly release)
<aaime> Geoserver release means gt2 release too
<aaime> 2.3.3
<jgarnett> I would like to get this factory finder stuff sorted out first.
<jgarnett> Wow a lot going on.
<jdeolive> yup
<jgarnett> We may have fun keeping all the patches straight; email early and often.
<jdeolive> yeha... so jody shall we wait to talk about 2.5-M0 until agenda item 4?
<gdavis> yes id like to get this factory finder stuff working before the release
<aaime> About 2.4.0-rc0, I just have to add the "dstype" param to the data source factories
<jgarnett> well not really; the bug will be fixed and the release will go out.
<aaime> to avoid confusion, just like the dbtype in jdbc datastores
<jgarnett> that is about it.
<jdeolive> aaime, will all the factories check for it?
<aaime> Yeah, that's the idea
<aaime> just like jdbc datastores
<jdeolive> i guess this is to stop property datastore for barfing exceptions
<aaime> ?
<aaime> I meant java.sql.DataSource factories
<aaime> At the moment we have two, DBCP and JNDI
<aaime> there is not confusion
<jdeolive> oh ok... sorry, i was confused
<aaime> but add one, say C3P0, and hell will break loose
<jdeolive> i understand now
<jdeolive> do you know when you will get that in andrea?
<aaime> I can do that as soon as tomorrow. Is that ok?
<jdeolive> sure
<jdeolive> i wasn't planning on releaseing to later in teh week
<jdeolive> ok, is that all for 1) ?
<jdeolive> ok. lets move on
<jdeolive> 2) query hints (again)
<jdeolive> aaime?
<aaime> Yeah, just wondering...
<aaime> when we'll ever deal with them
<aaime> I mean, 2.5.0 is focused on complex features and there is already enough mess generated by it
<jdeolive> what is the issue again?
<jdeolive> adding hints to the api?
<aaime> yep
<jdeolive> i see
<aaime> which may meean, break the Query API,or create a sub-interface
<aaime> if we do that before releasing 2.4.0 we can deal with this performance optimization in a stable branch
<jdeolive> sorry for asking dumb questions, but why do ew have to break the query api?
<aaime> we need to add a getHints() method to Query?
<jgarnett> we looked into using the existing transaction vendor specific hints
<jgarnett> it seems that this idea should be moved to "Query"?
<jdeolive> ok... but that does not break api? its an addition no?
<aaime> Transactions are not around during rendering....
<aaime> The implementations of Query will be broken by the addition
<aaime> I guess there is only one in the world
<aaime> but you know...
<jdeolive> fiar enough, its an api change regardless
<aaime> I just wanted to hear what people think about staying this slow for another six months
<jdeolive> well... this kind of addition we can add whenever we want...
<aaime> adding the decimation hints provided a 2x speedup on a raw test
<aaime> Not really, it's an API breakage
<jgarnett> did you try it with PostGIS and simplify?
<jdeolive> not client code
<aaime> Geoserver against a view that did decimation for a specific level
<jdeolive> i dont really consider breaking our single implementation or coupel of implementations an api breakage
<jgarnett> Andrea if you want to add this to 2.4
<jdeolive> anyways. i am fine with adding this to 2.4
<jgarnett> and get it in before justin makes the release this weekend
<aaime> So you would be for extending Query, right?
<jdeolive> does geoserver or udig impelment query directly?
<jgarnett> or based on your experience we can add it to the GeoAPI Query interface, as something we have learned is needed.
<aaime> I can try to setup a very quick proposal for this and have just the interface extension
<jdeolive> but you are right... its an interface... so its a breakage
<jgarnett> We do sometimes; I have found a lot of uDig code has created hacks for this kind of thing
<jgarnett> (mostly as workarounds)
<aaime> (you mean, for the lack of hints?)
<jgarnett> for lack of hints
<jgarnett> usually it is for lack of control on the creation side (ie no Hints for FeatureFactory)
<aaime> Ok, so, tomorrow morning I'll setup a quick proposal, if you can examine and vote it we can have it in for rc0
<jgarnett> yep
<jdeolive> sounds good
<jgarnett> thanks andrea.
<jdeolive> shall we move on
<aaime> Just one thing: a hint is a hint, a datastore is free to disregard it, right?
<jdeolive> definitley
<jgarnett> Correct
<aaime> Ok, good, then it's a painless addition
<jgarnett> It should however report what Hints it was able to use
<aaime> and we can start rolling in implements
<aaime> How?
<jgarnett> (as gdavis and myself learned to our peril)
<jgarnett> their was a method
<jgarnett> getImplementationHints
<jgarnett> in GeoTools Factory
<jgarnett> that we needed to implement or everything would break.
<gdavis> public Map getImplementationHints()
<gdavis> return Collections.unmodifiableMap( hintsWeCareAbout );
<aaime> Sorry, in which factory? You mean, the datastore factory?
<jgarnett> How you want to make use of this idea with Query I am not sure
<gdavis> this is from Factory I believe
<jgarnett> but the idea is there, if you pass in hints - you need to tell the user what hints were used.
<aaime> Ok, but the only factory I have around is the datastore factory
<jgarnett> there is a geotools interface called Factory
<gdavis> is Factory it's parent?
<jdeolive> i dont think its necessary that supported hints be advertised
<jgarnett> Not a lot of people know about Factory
<jgarnett> so I do not think the DataStore factory knows
<jgarnett> or even plays with this Hints idea.
<aaime> jgarnett, this can be the thing that stops me....
<aaime> It's too late to modify all of the datastores
<jgarnett> Andrea is talking at a different spot - ie Query is ment as a target to FeatureSource right?
<jgarnett> so that returns a FeatureCollection
<jgarnett> better put a getImplemenationHints() method on FeatureCollection?
<aaime> Hmmm.... yes
<jgarnett> FeatureSource.getFeatures( Query ): FeatureCollection
<jgarnett> food for thought.
jdeolive thinks you are making it more complicated then it needs to be
<aaime> Ah, yeah, that may be a place... FeatureCollection has already that many methods that one more won't break it singnificantly further...
<jgarnett> It is needed; because if you cannot respect the hints a user like Jesse is going to wrap the FeatureCollection (in order to make the result implement the interfaces he asked for). This is the kind of thing that uDig did when FeatureFactory could not be used.
<jgarnett> cool.
<aaime> jdeolive, for decimation it's not significant, you're right
<aaime> but if you provide a geometry factory, then you probably want to know if it was used
<jdeolive> fair enough...if you depend on ti then its not a good thing to hint
<jdeolive> imho
<jgarnett> jdeolive++
<aaime> Yeah, looks more like and order than a hint
<jgarnett> Hints really seems to be more than Hints IMHO
<jgarnett> ie the work gdavis is doing must be handled with a Hint (because that is all we got)
<aaime> Well, in most practical cases you can wrap and instanceof the elements that get returned
<jgarnett> but it is not "optional"
<aaime> and convert only if needed
<aaime> Yeah, in CRS subsystem hints are not just hints, either
<aaime> if I ask for a lon/lat factory, I'm requesting it, not suggesting it....
<jgarnett> okay we will have time for that rant in a bit ...
<jgarnett> andrea will make a proposal ... what is next?
<jdeolive> yup, we have abused hints... shall we move on
<jdeolive> 3) PrePostFilterVisitorSplitter confusion
<jdeolive> i think this is sfarber
<jgarnett> Where are the docs?
<aaime> grrr... why are datastores being created in udig directly?
<sfarber> docs:
<jgarnett> Because they wanted access to the connection pool
<jgarnett> so they could look at the metadata in the catalog handle
<jgarnett> (ie the info object)
<jdeolive> jgarnett, aaime, off topic!!!
<jdeolive> sfarber, continue
<jgarnett> jgarnett--
<simboss> hi guys, I am a bit late :-P
<sfarber> I'm guessing this is relevante:
<sfarber>
<sfarber> http://docs.codehaus.org/display/GEOTOOLS/SQLEncoder+Upgrade+to+GeoAPI+Filters
<sfarber> What, in particular, is a question you were trying answer that wasn't covered by the 2.4.x upgrade docs?
<sfarber> There was some chatter about a rename of the PPPFSV class, but I'm not sure where that originated.
<aaime> I would like to see a little explanation of how filter splitting works
<aaime> (may be offtopic too )
<sfarber> Not too be too cheeky, but there's a good explanation here:
<sfarber> http://gtsvn.refractions.net/geotools/trunk/gt/modules/library/main/src/main/java/org/geotools/filter/visitor/PostPreProcessFilterSplittingVisitor.java
<sfarber> at the top, from lines 92 to about 130
<sfarber> It's implementation-specific, though.
<sfarber> It's worth pointing out, also, that I didn't actually write the PPPFSV class, I simply made it work with the new GeoAPI filters.
<aaime> I see, never noticed it because I still live on 2.3.x and just forward port to trunk
<sfarber> If there are bugs and bad logic in it though, I'm more than willing to claim that I caused tham and fix them!
jdeolive is scared of that class
<jgarnett> Setting up a wiki page for you guys to take notes:
<jgarnett> - http://docs.codehaus.org/display/GEOTDOC/PostPreProcessFilterSplittingVisitor
<sfarber> yeah, it's not super-intuitive, but once you get over the stack-processor stuff, it's not super-unfriendly.
<jgarnett> (nice javadocs)
<jdeolive> yeah... its the stack processor stuff i dont quite get
<jdeolive> like what the various state of each stack represents... maybe i am missing somethign obvious
<jdeolive> anyways... off topic, bad jdeolive
<jgarnett> So rather than understand it; can we document with an example? ie What needs to be done to use this for PostGIS, DB2 etc..
<aaime> Tday I was bitten by the fact jdbc datastores are forced to declare capabilities in both the new and old sintax... that is, there is no bridge
<sfarber> aaime: YES! You're totally right about that.
<aaime> The migration page should talk about this
<sfarber> I think it's mentioned as an open question on that page I referenced a while ago.
<aaime> Yes, it is (still open)
<sfarber> under the heading 'the problem of FilterCapabilities'
<sfarber> So there were some problems that touched on this class this week.
<sfarber> Now that I'm back in front of my computer, I'm more than willing to deal with them, and try and actually sort out what's going on that's causing blow-ups in this class.
<sfarber> I've got emails out to Gertjan and now Aaime has bumped up against the filtercaps problem...so I'll be sure to keep the wiki up to date with any changes, and also try and run through gertjan's problem.
<sfarber> I think that's a wrap on this topic from my end. If anyone else's got questions about it, let's take them off-line?
<aaime> Great
<jdeolive> cool, thanks saul
<jdeolive> 4) factory finder stuff
<jdeolive> gdavis
<gdavis> right
<gdavis> I still cant get factory finder stuff to work (ie: metadata/org.geotools.factory stuff)
<gdavis> I've commited my recent changes that follow Martin's suggestions (as per the recent geotools dev list emails), but it still can't find my factory.
<gdavis> im beginning to think the factory finder stuff isnt working as it is suppose to
<jdeolive> unfortunatley it is written as a framework for martin, not just a framework
<simboss> that was a good one
<jdeolive>
<gdavis> finishing a geombuilder for the geometry module is the last thing in my way of getting this supported
<gdavis> the geombuilder needs a factory finder
<jdeolive> well i am not sure many of us can help you... looks like you might be stuff with debugger and emailing martin
<jdeolive> stuff = stuck
<gdavis> yah, have been at that for a while, but will continue i guess
<jgarnett> on the bright side
<jgarnett> I got stuck here back in febuary
<jgarnett> but now with these geometry classes it is much easier to debug
<jgarnett> (which is not to say fun)
<jdeolive> ok... shall we move on to the last agenda item
<gdavis> easier to debug, but you are still debugging it because it doesnt appear to work, right?
<jgarnett> It has now been a couple of weeks since the geomtry module was ready as an implementation
<jgarnett> I had a question of process for justin
<jdeolive> sure
<jgarnett> Is there anything stopping us from moving geometry over to supported status? It passes all the geotools supported things.
<jgarnett> Adding in this API change for GeometryFactory is a seperate matter.
<jdeolive> not that i see... i mean i dont understand why having a factory finder is a road block fo ryou guys
<jgarnett> Well we would like Geometry to be "normal" in the GeoTools sense.
<jgarnett> ie. It is not great for people to depend on these implementation classes directly.
<jdeolive> i would actually rather you dont add yoru geometry implementation into the mix... as i forsee problems
<jgarnett> how so?
<jdeolive> umm. the entire library assumes jts
<jgarnett> It still does
<jgarnett> well the entire library does not assume JTS
<jgarnett> referencing assumes DirectPosition.
<jdeolive> well i dont know of any concrete problems but i will bet you lunch that when you do hook it up there will be problems
<jdeolive>
<jgarnett> We are not trying to hook it up now; only make the implementation available.
<jdeolive> cool
<jdeolive> then what are we talkign about then
<jgarnett> Does the distinction make sense? We are not trying to use this with DataStore.
<gdavis> having it officially supported is the goal right now
<jdeolive> sure
<jgarnett> We are only making this implementation available (as alternative to jts-wrapper)
<gdavis> not hooking it up
<jdeolive> ok
<jgarnett> for projects that want ISO 19107
<jdeolive> ok, 5 mins left shall we move onto the last agenda item
<jgarnett> sure ...
<jdeolive> 5) feature collection
<jgarnett> 5) feature collection
<jdeolive> not sure who this is?
<jdeolive> ping jonathanv ?
<aaime> jonathanv, you are then one requesting this topic
<jgarnett> I want to remove FeatureList (as unloved)
<jonathanv> i didn't have anything specific
<jgarnett> that was quick then...
<jdeolive> cool
<jgarnett> A lot of docs are now available:
<jgarnett> http://docs.codehaus.org/display/GEOTDOC/05+Main
<aaime> Look, we (almost) all think it's broken, but trying to fix it it's more painful that keeping it as such...
<jgarnett> In particular: http://docs.codehaus.org/display/GEOTDOC/09+FeatureCollection+Iterator
<jgarnett> has a good comparison of Iterator, FeatureIterator and FeatureVisitor
<jdeolive> i agree with andrea
<jgarnett> (with FeatureVisitor being the easiest)
<jdeolive> especially since all of featureCollection is broken imho
<aaime> FeatureVisitor is new to me
<jgarnett> I have chatted with Justin about this a bit; I think what we all lack is time
<jonathanv> you all agree it's broken, but fixing it would be worse?
<aaime> why 3 ways to do the same thing?
<jgarnett> why? It has been around since GeoTools 2.2. ?
<aaime> breaking the api another time it's painful
<jgarnett> FeatureIterator is for Java 1.4 code
<jgarnett> it has been around since GeoTools 2.0
<jgarnett> it lets you get by without casts or something?
<aaime> It's a bad idea from whatever point of view you look at it, I already elaborated enough on this
<jgarnett> FeatureVistior is used for aggregation functions and so on ..
<aaime> and I'm tired to discuss, that's why I say let's keep it this way
<jgarnett> hrm.
<jonathanv> this document about iteration is great, prior to reading it i didn't know how to iterate through a collection
<jgarnett> If you guys can agree on what you want
<jgarnett> I would rather fix it then listen to complaints for the next four years
<jgarnett> jonathanv the javadocs have the same information.
<aaime> Simple, not have that thing implement Collection, remove all the extra methods, remove the plain iterator
<jgarnett> okay
<jdeolive> aaime++
<jgarnett> I would like to keep the visitor andrea
<aaime> that's what I've been saying since November and I'm tired of repeating it
<jgarnett> and the FeatureCollectionType
<jgarnett> okay; so if those are our marching orders then lets do it
<aaime> besides, we're arealdy breaking the API with the feature model, how many changes a certain set of users can tolerate?
<jgarnett> now is the best time (during the transition to GeoAPI)
<jgarnett> Andrea can I write up that as a proposal; please.
<jonathanv> hey, the users can barely tolerate what you have without even breaking it
<jgarnett> Jonathanv you are correct
<jgarnett> but we have not been able to implement what we have
<jgarnett> and although I try to document this stuff and take a users point of view as often as possible
<jgarnett> I need andrea and justin to be happy
<jdeolive> can we also add to teh mix that there are 5 different base classes for feaure collections
<jgarnett> since they are implementing
<jdeolive> we need to fix that too
<jgarnett> jdeolive++
<jdeolive> imho i would also like to see FeautreCollection implementing Feature go by ebye as well
<jgarnett> Okay I am going to write this up; as a proposal for GeoTools 2.5
<jdeolive> since that is another thing that we dont do properly
<jgarnett> I feel much more strongly about FeatureCollection implementing Feature
<aaime> jdeolive++
<jgarnett> than I do about it implementing Collection
<jdeolive> i know you do
<jgarnett> (sorry guys)
<jdeolive> but at least if we dont implement collection we can have a base class that actually impemenets feature properly
<jdeolive> and never worry about it again
<jgarnett> I understand why you feel that way; but I cannot really back down on this one.
<aaime> it's confusing, most of the time it's just a data holder, not a Feature
<jgarnett> jdeolive++
<aaime> can't we have a subclass for the specific case where it's a feature too?
<jgarnett> aaime that is more about an accident of our implementation sucking
<jgarnett> than anything else
<aaime> No, that's not it
<jonathanv> collections shouldn't be features
<jgarnett> yes they should
<aaime> when I ask a datastore for data
<aaime> I get a collection of features
<aaime> that's not a feature in any way
<jgarnett> It is in the strict sense
and I could be happy anyways
<jgarnett> it is :
<jgarnett> a) somethign you can draw on a map
<jgarnett> b) it is something that has a type
<jgarnett> c) it has attributes (bounds, etc...)
<jdeolive> not in the simple case it doesn't
<jgarnett> FeatureCollection is also used to model associations in the larger context
<jdeolive> and by simplei mean 99.9% of the time
<aaime> bah, this is just a way to look at a building and say it's like a toilet
<jgarnett> bringing together a group of Features according to some criteria or relationship)
<aaime> they have structural similarities, it does not mean they are the same
<jgarnett> so we have two uses cases
<jgarnett> both are important to me;
<jgarnett> aaime++
<jdeolive> the only argument for this that i see is that gml models featurecollection as a feature
<jgarnett> FeatureCollection has a very strict definition ...
<jdeolive> but even in that case... the only thign that acutally uses it (wfs) doens't make use of any of those modelling capabilities
<jdeolive> except for bounds
<aaime> So let's have a FeatureCollection subinterface that is a Feature
<aaime> or have a FeatureCollection superinterface that is not a feature
<aaime> simple things for simple and common cases please
<jgarnett> not intersted; I do not belive we define these constructs guys
<jgarnett> well we are always going to be limited to these common cases
<jgarnett> can we think ahead please.
<jgarnett> the only reason we have these common cases is because that is what we are limited to right now?
<aaime> Yes, bu having specific implementation that is a Feature and that gets built when a specific hint is provided, maybe
<jgarnett> if we want to sepearte out "Query" from FeatureCollection that is fine
<aaime> jgarnett, give me a significant use case where you do really need it to be a feature
<jgarnett> Do you remember your versioning postgis andrea?
<aaime> Yes
<jgarnett> much of what you get back there I would like to see as a FeatureCollection
<jgarnett> ie the collection of road=x, all versions
<jgarnett> the collection of features in this time perioid etc...
<aaime> it would work only with an svn like versioning support
<jgarnett> each of those resutls should have information about the collection
<aaime> with a cvs like there is not such a thing as a version that can be specified for the collection
<jgarnett> the other examples are with associations
<jgarnett> we are asked to do joins all the time
<jgarnett> I think it is a mistake
<jgarnett> we should have associations
<jgarnett> and let people navigate them
<jgarnett> producing feature collections along the way
<jgarnett> (see AssociationType used in FeatureCollectionType as an example of this)
<aaime> that still does not make them look like features
<aaime> in hiberante you can do all of the above
<jgarnett> and how does it not?
<aaime> yet everything is just a plain collection for the user
<jgarnett> I am not really interseted in hibernate in this context
<aaime> it's not a magical "hibernatebean" implementation
<jgarnett> I am interested in what feature collection is used for?
<jgarnett> capturing sets of features by spatial constraint, aggregation or association
<jgarnett> we mostly focus on spatial constraint
<jgarnett> because our model lacks aggregation and association
<aaime> I still don't follow you, you're polluting the most common case with stuff that may be needed in future and that does not need Feature anyways
<jdeolive> so jody
<jgarnett> I am trying to say that this stuff is needed now
<jgarnett> and time is passing us by
<jdeolive> you are willing to make feature collection impossible to implement just to be able to handle a theoretical use case that will probably never actually realize
<aaime> .... then I'm retarded
<jdeolive> seems a pretty high price to pay for me
<jgarnett> I am trying to ask what we can do to simplify thigns to the point we can implement
<jgarnett> I am more willing to give up java Collections
<jgarnett> than I am willing to give up the concept of FeatureCollection
<jgarnett> (because if we give it up we are stuck with assumptions again)
<jdeolive> jody your argument would be logical if there was actual code that used FeatureCollection as a feature
<jdeolive> i have a compromise guys
<jonathanv> sorry, i had to go do something
<jdeolive> and stick with me for a sec
<jgarnett> well do I implement that code?
<jgarnett> and just not have it work right now?
<jgarnett> okay.
<jdeolive> sure jody, you are right on how you want to model features
<jgarnett> (aside: I am happy to have this discussion, I am taking a hard line here because I want action - not because I am upset or anything)
<jdeolive> but what you are describing is not a good fit for the datastore api
<jgarnett> that is fine justin; then we should not use it for the datastore api
<jdeolive> so why dont we kill it from datastore api, use what we need
<jdeolive> and keep feature collection to be a feature
<jgarnett> perhaps we should just return Collection<Feature> from the DataStore api?
<jdeolive> nope, again collection is too hard to implmenet
<jonathanv> why not make a good collection, and a seperate adapter to make it a feature
<jgarnett> or go back to FeatureResults with a visitor (andrea I know you like FeatureReader but I find FeatureVisitor much easier to code against)
<aaime> then why there is nothing using FeatureVisitor?
<aaime> (I don't even know how it likes, I hear about it for the first time)
<jgarnett> jonathanv - you suggestion is good (and we have tried it). Difficulty is that users expect a colleciton to be in memory - we cannot seem to ask them to close their iterators.
<jonathanv> hmm
<jonathanv> so we fix their code for them
<jgarnett> basically users see a collection and start making assumptions.
<aaime> that's why I like FeatureIterator better, it has an explicity close method and throws exceptoins
<jgarnett> well we could put in a finalizer to throw an exception when they do not close their iterator
<jgarnett> But the idea is just bad; the normal java for loop does not work so well in this case and so on.
<jgarnett> in java 5 we can make the iterator() method
<jgarnett> return a FeatureIterator
<jgarnett> (and indeed I was thinking of doing just that)
<jonathanv> i use foreach loops
<jgarnett> (still amazed andrea has not seen FeatureVisitor)
<aaime> I'm looking at some code using it now, don't like it
<jgarnett> if you use for loop then there is a chance you will not make it to the end of the loop; at which point your iterator would not be closed - and it would be "game over"
<aaime> pretty much dumb and error prone
<jgarnett> compared with the other examples on this page: http://docs.codehaus.org/display/GEOTDOC/09+FeatureCollection+Iterator
<jgarnett> I like it quite a lot
<jgarnett> how so andrea?
<aaime> seeing code like "} else if (visitor instanceof UniqueVisitor) {" in a collection
<jonathanv> foreach loops? when do they not make it to the end?
<aaime> that does not use the provided class
<aaime> jonathanv, when you break out of them
<jgarnett> visitor is the alternative to iterator; the two get the same job done. Visitor leaves more responsibility in the hands of the data structure implementor - it is a good fit?
<aaime> getting back, this "} else if (visitor instanceof UniqueVisitor) {"
<jonathanv> i wouldn't use it if intended to break the loop
<aaime> is dangerous, what if I implemented my own UniqueVisitior with extra logic and you optimize it out
<jonathanv> don't put out hobbled shit because you think your users are stupid
<aaime> jonathanv, our gt2 code is full or break statements
<aaime> and we're still chasing up places that do not close iterators
<aaime> after 2 releases
<jonathanv> i say gt should fail in a spectacular fashion when you do something like that
<aaime> nope, it fails after load
<aaime> when file descriptors, connections or other limite resources are exhasted
<aaime> it may be in a minute, or in a month
<jonathanv> maybe it should delete all the files on your disk, so it's very clear that your code is buggy
<aaime> Bah
<jonathanv> i'm just saying: you need to stop trying to cover for user's mistakes, and you need to stop trying to prevent them from making them
<aaime> Here I'm upset because we're luring them into error, not because we don't shield them enough
<aaime> I know that when I'm using a stream I have to close it
<jgarnett> I understand andrea,
<jonathanv> it's painfully clear that you can't do either in an API
<jgarnett> would the finalizer idea help? It helped for transaction
<aaime> but if you give me a java.util.Collection I'll assume I can play with it clearly
<jgarnett> (more than you know - )
<aaime> it's still a hack
<jgarnett> I know.
<aaime> and when you see a reader was not closed, you just know the reader has been just garbage collected
<jgarnett> But it makes the problem visible; which is where are difficult is.
<aaime> it may have been instantiated one hour ago for what I know
<aaime> No, the difficulty is spotting the error
<aaime> and it's still there
<jgarnett> FeatureVisitor does not have this problem
<jgarnett> (similar to how closures are done in Ruby)
<aaime> you cannot bail out of it, not very useful
<jgarnett> that is not always true
<jgarnett> some visitor implementations let you return true/false to indicate the "bail"
<aaime> that visitor implementation does not have a boolean return value to bail out
<jgarnett> our catalog visitors did that
<aaime> Anyways, the most gross thing I've seen is optimizing visitors away with instanceof
<aaime> it ask vendetta from an OO point of view
<jgarnett> not sure I understand?
<jgarnett> the collection recognizing a vistor and producing the result of say Sum using alternat means?
<aaime> http://rafb.net/p/bdESNZ43.html
<jgarnett> that is fair use for a visitor ...
<aaime> No it's not
<aaime> if I implement my own version of MinVisitor that does extra work you should not optimize it away
<jgarnett> wow they needed some interfaces here
<aaime> that would have worsen things out
<aaime> at least with a class I can check the class is exactly MinVisitor
<aaime> (instead of using instanceof)
<jgarnett> fair enough
<acuster> how do you check?
<jgarnett> okay for this topic I will produce a proposal; and we can take this up if you guys are interested in it.
<aaime> visitor.getClass().equals(MinVisitor.class)
<jgarnett> At the very least it will let us record good ideas.
<aaime> k
<acuster> thanks
<jgarnett> visitor.getClass() == MinVisitor.class
<aaime> classloaders may play tricks on you if you use ==
acuster would have gone jody's route
<acuster> and learns a lesson from andrea
<aaime> not sure which is better
<jgarnett> Thanks for the discussion guys; once again sorrty for drawing such a hard line I hold your viewpoint in the highest regard; if not I would not keep revisiting it to try and find a way for us to go forward.
<jgarnett> aaime++ (sigh about the class loaders)
<acuster> what's the take home lesson? FC stays and everyone hates it?
<aaime> For 2.4 for sure
<aaime> for 2.5, don't know
<aaime> everyone - 1 hates it
<aaime> logs?
<jgarnett> acuser the take home leason is I document a propsal and everyone hates me
<jdeolive> i am on the logs
Labels