469,129 Members | 1,735 Online
Bytes | Developer Community
New Post

Home Posts Topics Members FAQ

Post your question to a community of 469,129 developers. It's quick & easy.

Closures for undo: bad idea?

I'm working on an application that does some DOM manipulation. I want
to add a mechanism for "Undo" so that the user can revert to the
previous state after performing a mistaken action. Simple idea, not
necessarily simple to implement.

My first idea for this is to use closures and an array that tracks the
inverse of the user's actions. Here's a simple example in which the
user can only remove DOM nodes and then undo the removal repeatedly
until they've reached their desired state (or the original state):

var App = {

undo: [],

removeNode: function(node)
{
//remove the node
var parent = node.parentNode;
var nextSibling = node.nextSibling;
parent.removeChild(node);

//inverse action goes into undo buffer
App.undo.push(function() {
if(nextSibling)
parent.insertBefore(node, nextSibling);
else
parent.appendChild(node);
});
},

undoLast: function()
{
if(App.undo.length)
(App.undo.pop())();
}
};

(Obviously the removeNode and undoLast functions are invoked from
elsewhere in response to user actions.)

My concern is with the closures created in the removeNode function.
This enables the undo to take place quite easily, but I am worried about
memory leakage.

Let's say the user undoes a few actions, and then performs a few more
actions. Are the nodes associated with the actions that were undone
still around in memory? There is now no way to access the nodes, but
will they be garbage collected or are they just leaking memory like a sieve?

Has anyone implemented undo this way? Any other strategies I should
consider?

Thanks,
Jeremy
Jun 27 '08 #1
1 1462
Jeremy,
It's difficult to be definitive about this and the outcome might well be
browser dependent.

On one hand, when the use performs an undo, your undo function is pulled
of the undo stack and executed. After that,
there are no outstanding references to the function and you would think
that the garbage collector wold clean up the function and in turn
release references to state variables (such as nextSibling) help by the
closure.

On the other hand, I've been caught with closure induced memory leak
esp. in IE before and so I would probably use a slightly different
approach. Still keep the undo stack but change what you store on it.
Instead of pushing a closure, push an object containing arguments and
a reference to a statically generated function to call to effect the
undo.

The undo function facility is invoked like the following then:

var App = {

undo: [], // an array of "undo" Objects

undoLast: function()
{
var oUndo = App.undo.pop();

if ( oUndo !== undefined) {
oUndo.fnUndo.apply( oUndo.thisp, oUndo.args);
}
}

};

What you need to complete is the code to record the undo Objects.

Cheers
Justin Johansson

Jeremy wrote:
I'm working on an application that does some DOM manipulation. I want
to add a mechanism for "Undo" so that the user can revert to the
previous state after performing a mistaken action. Simple idea, not
necessarily simple to implement.

My first idea for this is to use closures and an array that tracks the
inverse of the user's actions. Here's a simple example in which the
user can only remove DOM nodes and then undo the removal repeatedly
until they've reached their desired state (or the original state):
<cut/>
>
(Obviously the removeNode and undoLast functions are invoked from
elsewhere in response to user actions.)

My concern is with the closures created in the removeNode function. This
enables the undo to take place quite easily, but I am worried about
memory leakage.

Let's say the user undoes a few actions, and then performs a few more
actions. Are the nodes associated with the actions that were undone
still around in memory? There is now no way to access the nodes, but
will they be garbage collected or are they just leaking memory like a
sieve?

Has anyone implemented undo this way? Any other strategies I should
consider?

Thanks,
Jeremy
Jun 27 '08 #2

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

3 posts views Thread by Enigman O'Maly | last post: by
1 post views Thread by black | last post: by
14 posts views Thread by Alexander May | last post: by
5 posts views Thread by paolo veronelli | last post: by
4 posts views Thread by Marc Tanner | last post: by
16 posts views Thread by Karl Kofnarson | last post: by
5 posts views Thread by Steve | last post: by
4 posts views Thread by king kikapu | last post: by
reply views Thread by zhoujie | last post: by
By using this site, you agree to our Privacy Policy and Terms of Use.