Jody Garnett ha scritto: >> Please don't add a layer.dispose() method, resources to be disposed >> are harder to use so let's limit the usage of dispose/close methods >> to cases where it's really necessary to do so (file, streams, >> database connections and so on). Swing does not have a "dispose()" >> method for components despites handling lots of listeners, so >> there's something to learn there I guess (worst case we can just >> have the listeners be dropped in the finalizer?). > > I remember what you were saying about Swing using weak references for > its listeners; currently I am using the CopyOnWriteArrayList > implementations. I will move the clean up code to a dispose method > and see how we go.
Which is basically like saying: "I remember what you said but I did not look into it, and I'm also ignoring what you said above too, and adding dispose methods all around". If you think my suggestion is bad at least say so directly (and explain why). I'm not saying we don't need dispose() method, I'm asking if we can avoid them and make the API easier to use (or else, not harder to use than it was, since before we did not have those dispose methods). If there is a solid reason and the links I provided do not work so be it, but at least let's discuss this? >> In any case, I'm ready to be convinced that disposal methods are >> really necessary, but please let's explore also this road before >> adding complexity to the API: >> http://www.google.it/search?q=swing+listeners+weak >> >> Btw, Map also holds a listener list but does not have a dispose >> method (it's not like I want one, but things should be consistent). >> >> > So if we did not have a dispose method what would be the correct > corse of cleanup for a interactive application? Do we expect them to > clean up their feature sources? Probably not since they can clean up > their datastrores which they should already be tracking. Definitely not > Do we expect them to clean up after the GridCoverage readers? If so > how ... A map context that closes readers might be problematic, what if the underlying application caches the readers? Hum, it's a connection pool like situation, I guess we would have to wrap them so that the close method does nothing (and kiss goodbye the ability to recognize their actual type by instanceof... ). > You will noticed I made all the event listener code very very lazy so > nothing will be fired or need to be cleaned up unless some client > code decides to actually listen. In fact I did not complain about that. I'm talking about dispose() method here, not about lazy event firing. >> DirectLayer wise I would really love to see at least one >> implementation to make sure we're not just pulling the interface >> for it out of... a magician hat ;-) > > Agreed; I was thinking of one showing CRS bounds. > >> I believe a direct layer could be both a customized renderer that >> does geographic data or a simple decoration over a map, not sure a >> single interface can accommodate both (maybe it does, but better to >> make sure). > > The javadocs indicate both uses cases; I am confident it does due to > experience in uDig which has the same setup. > >> A copyright layer (decoration) and wms layer (actual data as far as >> the interface is concerned) should cover it. > > The wms layer implementation is not working right now (on trunk even > before this change). I am not sure what is wrong with the WMSLab > and/or WMSLab2 tutorial code. I can see layers published by a remote GeoServer just fine? I'm getting a failure with the DMSolutions server, but they are declaring a fake EPSG code (one that is not in the EPSG database). Anyways, found a way to make it more solid.... just no idea if this is the error that you were seeing. I am the last one that worked on those cascaded WMS layers so I am ready to take the blame, but please give me something to act upon it and fix it. >> Oh, it is a bit odd to see addLayer and then addMapLayerListener, >> to keep things consistent it would be nice to have addLayerListener >> instead? But that would break the api or force into another set of >> wrappers. > >> Don't know... what do you think? > > Yeah I am at the turning point; I have done as much as possible > without breaking any api. The moment I start breaking api it is > tempting to remove the cruft. > >> Btw, I'm looking at the patch and there are reformats... were they >> really necessary??? > > In general I only reformatted when tabs were being used; I try to be > limited in such cases. > >> - TestUtilites.showRender("testLineLabeling", renderer, >> timout, env); + + >> TestUtilites.showRender("testLineLabeling", renderer, timout, env >> ); } > >> (note, afaik the java coding convention does not ask people to add >> space after ( and before ), I personally find that unecessary and >> distracting). > > Perhaps the settings we have in our repository for geotools > formatting is wrong? I am trying eclipse 3.6 out and they may of > messed something up. I will check. Jody, mixing reformats and behavior changes makes the patch harder to read. >> I also see code that has just been commented out like in >> FeatureSourceMapLayer, bad practice... well, unless you noted down >> all the code that you've commented out and will remove it later. > > That will be one of the last things I do; remember up to now is a > straight refactor. I will try and make the code clean and consistent > today. As long as you remember to remove the commented out code I have no issues with that either. >> MapContext wise I see the interface has been modified and one >> method removed. No issue for me, but didn't you say the old API was >> not being changed? > > I did not; the old api was deprecated - it was a setAreaOfInterest( > Envenlope bounds ) method that presents a conflict with the also > existing ReferencedEnvelope method; forcing people to cast each and > every time they call either method in order to be specific. I see, thanks for the explanation. Cheers Andrea -- Andrea Aime OpenGeo - http://opengeo.org Expert service straight from the developers. ------------------------------------------------------------------------------ ThinkGeek and WIRED's GeekDad team up for the Ultimate GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the lucky parental unit. See the prize list and enter to win: http://p.sf.net/sfu/thinkgeek-promo _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
