<br><br><div class="gmail_quote">On Fri, Jan 9, 2009 at 02:05, Tim Schaub <span dir="ltr"><<a href="mailto:tschaub@opengeo.org">tschaub@opengeo.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hey-<br>
<div class="Ih2E3d"><br>
Erik Uzureau wrote:<br>
> This is an interesting proposition.<br>
><br>
> I have wanted for a long time to reform the way we do destroy().<br>
> Specifically<br>
> I've wanted to go through and make a patch replacing all the<br>
><br>
> this.propX = null;<br>
><br>
> with<br>
><br>
> delete this.propX;<br>
><br>
> ...but have sort of just never gotten around to it.<br>
><br>
> as for the proposition here, two thoughts:<br>
><br>
> 1) overriding the valueOf() thing makes me a little nervous. It seems<br>
> like this is the main reason for you suggesting this in the first place,<br>
> and I can see how this would be handy... but my gut reaction is that<br>
> having to special case our code to deal with a variable that has<br>
> possibly been destroy()ed... it sounds like that's not a situation we'd<br>
> ever want to be in. the response to that might be "keep on dreamin' uz,<br>
> have you looked at the code lately?" and i'm happy to stomach that<br>
> ;-).... but seriously, in my mind, the destroy() call in my mind should<br>
> only be happenning when the variable in question is absolutely no longer<br>
> needed. which to me means that there should never be code calling it at<br>
> that point. maybe there are some cases i'm not thinking of (event<br>
> callbacks?) but anyways, just a gut feeling. as always, more than happy<br>
> to be convinced otherwise.<br>
<br>
</div>I think I get what you are saying. We'd have to dig around a bit to be<br>
able to characterize the circumstances where destroy has been called<br>
before we expect it to be called. I just know there are places where we<br>
are doing things like checking obj.events because we don't know if obj<br>
has been destroyed or not yet. My proposal lets us check (obj == false)<br>
instead. </blockquote><div><br>right, right. again, i'm not *totally* against this. it just still seems a little funny<br>
to me. like tweeking with the rules somehow. maybe i'm wack though. let's <br>
hear what others have to say. <br>
<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">Otherwise, it doesn't improve things.<br>
<div class="Ih2E3d"></div></blockquote><div><br>actually, i think even if it didn't have that bit, it would still be a helpful improvement.<br>god knows if there is actually any difference in memory, but the simple fact of <br>
replacing 343 million this.foo = null statements with a simple, clean, <br>OpenLayers.Object.destroy(this); I think is in and of itself a really nice improvement.<br>In the cases where we already try to be thorough about setting properties to <br>
null or deleting them, it makes the code simpler, shorter, and cleaner. In the cases<br>where we set to null, i think calling delete on them is more appropriate and more <br>complete. in the cases where we are *not* diligent, then it cleans out stuff that <br>
we forgot to clean ourselves. Overall, this is a big win. I like it a lot. <br><br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="Ih2E3d">
<br>
><br>
> 2) there is the issue here of objects-with-objects. like when you have<br>
> an obj 'foo' which has a property 'bar', which is also an object. what<br>
> to do in that case? do we want to make this recursive? base case (obj<br>
> has no properties or typeof obj is simple base type) return, and<br>
> otherwise recursively call OL.Object.destroy() on every (typeof object)<br>
> property? it goes without saying how that can be a dangerous black hole.<br>
> keeping it as is below, however, has the danger of orphaning (?) a bunch<br>
> of memory. is there any way to satisfy both?<br>
<br>
</div>The OpenLayers.Object.destroy function I proposed was not an alternative<br>
to the destroy methods we currently have. Instead it would be called at<br>
the end of the destroy sequence. Things would go like this: I clean up<br>
everything I am responsible for (possibly by calling<br>
this.foo.destroy()), then I call OpenLayers.Object.destroy(this). This<br>
is intentionally not recursive.</blockquote><div><br>gotcha. yeah, i like that. it's a good final sweeper. i like it a lot. <br><br><br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
>> anyways, destroy() reform is a fun topic and i'd love to hear what<br><div class="Ih2E3d">
> others have to say.<br>
<br>
</div>Yeah, also interested.<br>
</blockquote><div><br>word. let's see if anyone else is interested (probably not) :-)<br><br>e<br><br><br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
Tim<br>
<div class="Ih2E3d"><br>
> good suggestion, tim.<br>
> ups,<br>
> e<br>
><br>
> On Thu, Jan 8, 2009 at 18:56, Tim Schaub <<a href="mailto:tschaub@opengeo.org">tschaub@opengeo.org</a><br>
</div><div><div></div><div class="Wj3C7c">> <mailto:<a href="mailto:tschaub@opengeo.org">tschaub@opengeo.org</a>>> wrote:<br>
><br>
> Hey-<br>
><br>
> We don't currently have a uniform way of determining if an OpenLayers<br>
> object has been "destroyed". I'm wondering what people think of adding<br>
> something like this:<br>
><br>
> OpenLayers.Object.destroy = function(obj) {<br>
> for(var prop in obj) {<br>
> delete obj[prop];<br>
> }<br>
> obj.valueOf = function() {<br>
> return false;<br>
> };<br>
> }<br>
><br>
> Then we could use the following to decide if something has already been<br>
> destroyed:<br>
><br>
> obj == false // true after OpenLayers.Object.destroy(obj)<br>
><br>
> Benefits:<br>
><br>
> We'd have a uniform way of determining if the destroy method had been<br>
> called.<br>
><br>
> Drawbacks:<br>
><br>
> This only works for the level of type coercion done by the equality<br>
> operator. The logical not operator still considers the object truthy<br>
> (so !OpenLayers.Object.destroy(obj) === false). So, we'd probably still<br>
> end up doing things like this (to test undefined or destroyed):<br>
><br>
> if(obj && obj != false) {...}<br>
><br>
> Also, maybe we're better off checking explicitly for properties as we do<br>
> currently (!obj.events etc.)<br>
><br>
> Any opinions?<br>
> Tim<br>
><br>
> --<br>
> Tim Schaub<br>
> OpenGeo - <a href="http://opengeo.org" target="_blank">http://opengeo.org</a><br>
> Expert service straight from the developers.<br>
> _______________________________________________<br>
> Dev mailing list<br>
</div></div>> <a href="mailto:Dev@openlayers.org">Dev@openlayers.org</a> <mailto:<a href="mailto:Dev@openlayers.org">Dev@openlayers.org</a>><br>
<div class="Ih2E3d">> <a href="http://openlayers.org/mailman/listinfo/dev" target="_blank">http://openlayers.org/mailman/listinfo/dev</a><br>
><br>
><br>
<br>
<br>
</div>--<br>
<div><div></div><div class="Wj3C7c">Tim Schaub<br>
OpenGeo - <a href="http://opengeo.org" target="_blank">http://opengeo.org</a><br>
Expert service straight from the developers.<br>
_______________________________________________<br>
Dev mailing list<br>
<a href="mailto:Dev@openlayers.org">Dev@openlayers.org</a><br>
<a href="http://openlayers.org/mailman/listinfo/dev" target="_blank">http://openlayers.org/mailman/listinfo/dev</a><br>
</div></div></blockquote></div><br>