6

I have searched around for this but thus far have not been able to find a duplicate, I may be using the wrong keywords...

I am trying to temporarily change a function stored in an object, but am having trouble setting it back to what it was before.

Consider this:

// Set the options object
var options = {
    success: function(){
        console.log('Original Function Called');
    }
}

// Save the options
$('#foo').data('bar',options);

And then in another function:

// Get the options
var options = $('#foo').data('bar');

// Store the old options
var old_options = options;

// Temporarily change the success function
options.success = function(){
    console.log('Temporary Function Called');
}

// Save the options
// This allows the other functions to access the temporary function
$('#foo').data('bar',options);

// Do stuff here that uses the new options

// Reset the options to include the original success function
$('#foo').data('bar',old_options);

I would have expected that to only display Temporary Function Called once, however, it seems to completely replace the old success callback with the temporary callback.

Can anyone tell me why and how I can get around this?

UPDATE

I thought that extend would fix this but it seems that the issue may be a little deeper. I have decided to post a snippet of my actual code this time. Please be aware of the following before reading:

  1. SM is pretty much just an alias of jQuery, please ignore it.
  2. success and error are parameters supplied to the function

Here is my code:

// Get the properties
var properties = $(form).data('autosave');
switch(parameter){
    case 'save':
        var old_properties = $.extend({},properties);
        // Set the new callbacks if they have been supplied
        properties.options.success = typeof success!=='undefined' ? success : old_properties.options.success;
        properties.options.error = typeof error!=='undefined' ? error : old_properties.options.error;
        // Save the properties
        $(form).data('autosave',properties);
        // Call the save method before setting the interval
        SM(form)._as_save();
            properties = $.extend({},old_properties);
        // Save the old properties
        $(form).data('autosave',properties);
        // Clear the current interval
        clearInterval(properties.interval);
        // Call the save method periodically
        properties.interval = setInterval(function(){
        SM(form)._as_save();
        },properties.options.interval);
    break;
}
// Save the properties
$(form).data('autosave',properties);
Improve this question
5
  • You're saying that after $('#foo').data('bar',old_options); you are still seeing Temporary Function Called? Commented Jun 19, 2013 at 20:54
  • Some good answers down there. Commented Jun 19, 2013 at 20:56
  • I'm a bit confused - this new update, was that after our discussion of the shallow copy vs. deep copy? I see your success method is actually an options.success() method (and similarly for error). That is exactly the situation where a deep copy would be required. So are things still broken or did the deep copy do the trick? Commented Jun 20, 2013 at 2:42
  • @MichaelGeary No this came before :-) The deep fixed it Commented Jun 20, 2013 at 9:11
  • Whew! You had me worried. It makes sense now, looking at your code in the update: The shallow copy would have worked with your code in the original question, where the success callback was a first-level property of the main options object. But the actual code in your update is a bit different: it has a properties.options.success function - not a direct child object but two levels down. That's exactly the situation where a deep copy is needed (or at least a more specialized copy that would handle this nested object). Commented Jun 20, 2013 at 9:42

4 Answers 4

Reset to default
10

When you run this code:

var old_options = options;

you are not making a copy of the entire options object that you can restore later. You are merely saving a reference to the same object. In other words, old_options is the very same object as options, so when you assign a new value into options.success, you're changing it in both options and old_options—because they are the same object.

To fix this, you can use an object cloning function to make a copy of the object which you can then restore later. Since you're using jQuery, you can change the line above to:

var old_options = $.extend( true, {}, options );

Now, when you change options.success, you're only changing it in the options object. old_options is unaffected, so your later call will restore it successfully:

$('#foo').data('bar',old_options);

Interestingly enough, this may still work OK even if options.success is an asynchronous callback (which sounds likely from the name). That's because whatever code calls that .success() method later on, they should still be holding on to a reference to your modified options object—even if you've restored the old one back into the element's data in the meantime. At least one could hope for that; if the other code digs back into the $().data() to find the .success callback then you'd be in trouble.

The $.extend() call above does a "deep" (recursive) copy of the options object. That is, if one of the properties inside options is itself an object, it also clones that object instead of just copying a reference to it.

If you leave out the true argument, $.extend() does a shallow copy instead:

var old_options = $.extend( {}, options );

This still creates a new object and copies over all the properties from an existing object, but if one of those properties is itself an object it doesn't clone that object, it just copies a reference. This is more efficient if it works with the structure of the object you're using, otherwise you can use the deep copy.

If the properties/methods you need to save and restore are direct children of the main object, a shallow copy should be enough. Here's a case where you'd definitely need a deep copy:

{
    url: 'test',
    events: {
        success: function( data ) {
            // ...
        }
    }
}

Here we have an object with an events property, and that property is itself an object with some properties/methods of its own (in this example an events.success() method. If you do a shallow copy of this object, the original and the copy will share a common events object. So if you did something like this:

options.events.success = function(...) {...};

You'd actually be updating that in both options and old_options. No good. That's where a deep copy is needed.

Improve this answer
Sign up to request clarification or add additional context in comments.

15 Comments

That is what I thought I might be doing. Should I just use the jQuery extend method?
Perfect, thank you!! Wasnt too sure how to word the question but I thought this might be the reason. Thank you for your time
Its a little more complicated than that, thats why I didn't post my code. Think of it as a jQuery extension (it is more than this but not really relevant). If you call $('#foo').bar(options) it creates and instance of the bar. And then you can call $('#foo').bar('save',function(){}) which execute the save method and call the callback supplied just once and then revert back to the callback specified in the original instance options
It's funny, but I was adding that line of code with $.extend() at the very moment you were writing that comment asking about it! Plus of course all the other answers saying the same thing. I guess the old saying is at least sometimes true: great minds think alike. :-)
Hahaha. Irritating thing is that I was going to try extend() but didn't think it would have made a difference... Would have deleted this post as soon as I saw your answer, but as you got such a good load of rep out of it thought I would leave it :-)
|
3

The problem is that options has reference semantics:

// Store the old options
var old_options = options;

This comment lies. You do not have a copy of the old options; rather, you have another reference to the same options object (another name with which you can refer to it).

So when you overwrite options.success, this change is also visible on old_options. The code that uses .data to store and revert the value of the options is redundant.

The only thing you need to do is this:

var old_success = options.success;
options.success = function() { /* whatever */ };

// code that uses the new success callback

options.success = old_success; // restore original value
Improve this answer

2 Comments

@MichaelGeary has got it, but just one comment on your answer. I originally tried this method but it ended up executing options.success...
Even if the solution turned out to be different, you get a hearty +1 for this note: "This comment lies." That is such an important point: oftentimes a bug is caused because we think the code is doing what a comment says it does - whether that's an explicit comment or a mental "comment" - i.e. our [mis]understanding of the code. It's all too easy to overlook a case where we think the code is doing one thing but it really does another. I've done that many times, that's how I know :-)
2

When you do this var old_options = options you're not copying options values but a reference to it. From now on, old_options and options point to the same memory spot and changes on any of them affect to both.

Improve this answer

Comments

2

Your issue is you are working with objects. You are passing around references to the object. The underlying object is the same, all you have done is add a variable that points at the same address.

You will have to clone the object. EG create a new object with the same properties and copy them over.

This post may be useful to you.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.