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