Value inside of callback is not being updated on each function call

Question

I have a pretty simple function as shown below. This deletes a users slip after confirming that's what they want to do (note the custom confirm). Inside the callback function on confirm, id gets set correctly the first time, but is never updated after that.

If I console.log inside of the deleteSlip function, id is always correct. If I console.log inside of the callback, it's always the first id I send it. I see the same thing when I debug the javascript. The value is correct the first time, but never updates after that.

What do I need to do to get the value inside the callback function to update when it updates in deleteSlip?

function deleteSlip(id){
    //  id is correct right here
    confirm('This will delete your slip.  Are you sure you want to do this?', function(confirmed){
        //  id here is always the first value from deleteSlip, and never changes
        if (confirmed)
        {
          // do stuff 
        }
    });
}

The confirm being called is custom and is defined below:

window._originalConfirm = window.confirm;
window.confirm = function(text, cb) {
  bootStrapConfirm = function() {
    if(! $.fn.modal.Constructor)
      return false;
    if($('#windowConfirmModal').length == 1)
      return true;
    $('body').append(' \
    <div id="windowConfirmModal" class="modal hide fade" tabindex="-1" role="dialog" aria-hidden="true"> \
      <div class="modal-body"> \
      <button type="button" class="close" data-dismiss="modal" aria-hidden="true">×</button> \
        <p> alert text </p> \
      </div> \
      <div class="modal-footer"> \
        <button class="btn btn-danger" data-dismiss="modal" aria-hidden="true">No</button> \
        <button class="btn btn-primary" data-dismiss="modal" aria-hidden="true">Yes</button> \
      </div> \
    </div> \
    ');
    function unbind() { 
      $("#windowConfirmModal .btn-primary").unbind('click', confirm);
      $("#windowConfirmModal .btn-danger").unbind('click', deny);
    }
    function confirm() { cb(true); delete cb;}
    function deny() { cb(false); delete cb;}
    $("#windowConfirmModal .btn-primary").bind('click', confirm);
    $("#windowConfirmModal .btn-danger").bind('click', deny);
    return true;
  }
  if ( bootStrapConfirm() ){
    $('#windowConfirmModal .modal-body p').text(text);
    $('#windowConfirmModal').modal();
  }  else {
    console.log('bootstrap was not found');
    window._originalConfirm(text);
  }
}

deleteSlip is an onclick HTML attribute on a button. Like so:

onclick="deleteSlip(1234)"

Again, to reiterate, the id inside of deleteSlip is correct. The id inside of the callback on the confirm is whatever it gets first and never changes. The callback may be async, but I'm only calling it once at any given time. deleteSlip is fired on button click.

I posted this question before and was told it was too ambiguous. Please let me know what you need to see to help me work through this. The "do stuff" area has an ajax call, but the value is wrong before that (thus not showing it for people who don't read the entire question).

Update: It looks like this is an issue with the custom confirm. I change the custom confirm to be the following:

window.confirm = function(text, cb) {
  bootStrapConfirm = function() {
    if(! $.fn.modal.Constructor)
      return false;
    if($('#windowConfirmModal').length == 1)
      return true;
    $('body').append(' \
      <div id="windowConfirmModal" class="modal hide fade" tabindex="-1" role="dialog" aria-hidden="true"> \
      <div class="modal-body"> \
      <button type="button" class="close" data-dismiss="modal" aria-hidden="true">×</button> \
        <p> alert text </p> \
      </div> \
      <div class="modal-footer"> \
        <button class="btn btn-danger" data-dismiss="modal" aria-hidden="true">No</button> \
        <button class="btn btn-primary" data-dismiss="modal" aria-hidden="true">Yes</button> \
      </div> \
      </div> \
    ');
    return true;
  }
  if ( bootStrapConfirm() ){
    function confirm() { cb(true); }
    function deny() { cb(false); }
    $("#windowConfirmModal .btn-primary").unbind('click', confirm);
    $("#windowConfirmModal .btn-danger").unbind('click', deny);
    $("#windowConfirmModal .btn-primary").bind('click', confirm);
    $("#windowConfirmModal .btn-danger").bind('click', deny);
    $('#windowConfirmModal .modal-body p').text(text);
    $('#windowConfirmModal').modal();
  }  else {
    console.log('bootstrap was not found');
    window._originalConfirm(text);
  }
}

Now when confirm is called, it calls ALL the id's it's ever seen. Something I'm not unbinding properly?

I was told it was too ambiguous before. I've updated the question with much more info. — nwalke, Feb 27 '13 at 16:42
I'm just glad I made a copy of my comment that I couldn't post, so here it is... — the system, Feb 27 '13 at 16:43
The first time you call your custom confirm, you're creating a new dialogue element and setting up handlers that call your callback. I don't see any attempt to remove the element or to unbind the handlers after executed. I do see `delete cb;` inside the handlers, which will have no effect because using `delete` on a variable isn't a valid operation. So I can only assume that the original custom dialogue is still being used with the same original handlers which reference the same original callback. — the system, Feb 27 '13 at 16:44
So do I need to somehow recreate the `confirm()` and `deny()` calls inside themselves? Would you have an example for me of how to do that? — nwalke, Feb 27 '13 at 16:55
If you're unbinding them, then you need to rebind them on the next call. Right now, the code that does the binding/unbinding is within the same `if` statement as the code that creates the element. If you want to reuse the element, then only make its creation conditional, and make the binding happen every time. So probably best to put those functions in the `if ( bootStrapConfirm() ) {` statement. — the system, Feb 27 '13 at 16:59
Just to make sure I'm on the right page, you want me to move the bindings and the `unbind` function to the `if ( bootStrapConfirm() ) {` statement? — nwalke, Feb 27 '13 at 17:04
Yes, but then I don't know how the rest of the code works. So given that the dialogue is properly displayed and removed, then I'd move all the functions/bindings to that statement so that they're remade every time the `confirm` function is invoked. — the system, Feb 27 '13 at 17:08
That works! Now I'm seeing too many ids, let me update my question with what I have. — nwalke, Feb 27 '13 at 17:10
http://stackoverflow.com/questions/111102/how-do-javascript-closures-work — peterfoldi, Feb 27 '13 at 17:15
Put the `.unbind()` code back into the `function unbind() {...`, and then call it from inside the `confirm/deny` functions. So you're basically replacing the `delete cb;` that you had with `unbind();`. — the system, Feb 27 '13 at 17:15
@tubaguy50035: Be careful with the accepted answer to that question. It's not very good or accurate. — the system, Feb 27 '13 at 17:20
@thesystem I put the `function unbind` back after the `$('body').append`. But that takes it out of scope for the `confirm` and `deny` functions, I get `unbind is not defined`. Should `confirm` and `deny` go back after the `$('body').append` as well? If so, doesn't that take it out of the scope of the `if ( bootStrapConfirm() ){ `? — nwalke, Feb 27 '13 at 17:23
All the functions that need to deal with your `cb` need to all be in the same scope. Put them all in the `if ( bootStrapConfirm() ){`. Or you could put them outside of any `if` statement if you want as long as they're not inside the `bootstrap` function. — the system, Feb 27 '13 at 17:29

fredrik · Accepted Answer · 2013-02-27T17:06:18.367

You have a scope problem here. id is defined in deleteSlip(), not in the callback. Therefor you should pass id as a parameter to the callback.

In a built-in jquery this would be done by adding parameters just before the callback function is given to confirm(). Not sure how you would have to do it though.

In your case I would do it like this:

function deleteSlip(id){
  //  id is correct right here
  confirm('This will delete your slip.  Are you sure you want to do this?', id, function(cb_id, confirmed){
      //  id here is always the first value from deleteSlip, and never changes
      if (confirmed)
      {
        // do stuff 
      }
  });
}

And:

window._originalConfirm = window.confirm;
window.confirm = function(text, id, cb) {
  bootStrapConfirm = function() {
    if(! $.fn.modal.Constructor)
      return false;
    if($('#windowConfirmModal').length == 1)
      return true;
    $('body').append(' \
      <div id="windowConfirmModal" class="modal hide fade" tabindex="-1" role="dialog" aria-hidden="true"> \
      <div class="modal-body"> \
        <button type="button" class="close" data-dismiss="modal" aria-hidden="true">×</button> \
        <p> alert text </p> \
      </div> \
      <div class="modal-footer"> \
        <button class="btn btn-danger" data-dismiss="modal" aria-hidden="true">No</button> \
        <button class="btn btn-primary" data-dismiss="modal" aria-hidden="true">Yes</button> \
      </div> \
    </div> \
    ');
    function unbind() { 
      $("#windowConfirmModal .btn-primary").unbind('click', confirm);
      $("#windowConfirmModal .btn-danger").unbind('click', deny);
    }
    function confirm() { cb(id, true); delete cb;}
    function deny() { cb(id, false); delete cb;}
    $("#windowConfirmModal .btn-primary").bind('click', confirm);
    $("#windowConfirmModal .btn-danger").bind('click', deny);
    return true;
  }
  if ( bootStrapConfirm() ){
    $('#windowConfirmModal .modal-body p').text(text);
    $('#windowConfirmModal').modal();
  }  else {
    console.log('bootstrap was not found');
    window._originalConfirm(text);
  }
}

Disclaimer: Havn't run the code, so some error could be there. But the idea should be sound.

Yes it does, but the closest scope has precedence. You could name it cb_id you want to be sure. — fredrik, Feb 27 '13 at 16:54
@fredrik: It isn't declared in OP's callback, so when `id` is used in the callback, it'll use the one from the enclosing `deleteSlip` scope. — the system, Feb 27 '13 at 16:55
I stand corrected, updated the code to something that should work better. Can't argue when to people make the same comment after all :D — fredrik, Feb 27 '13 at 17:06
That still won't work since it doesn't address the actual issue. The issue was that the original callback was permanently bound. With your solution, that issue still exists. Only change is that the original callback will be passing the original ID on every invocation. In other words, you're fixing the part that wasn't broken. — the system, Feb 27 '13 at 17:36

score -1 · Answer 2 · answered Feb 27 '13 at 16:45

-1

You should pass the id explicitly to callback function.

    function deleteSlip(id){
        //  id is correct right here
        confirm('This will delete your slip.  Are you sure you want to do this?',        function(confirmed,id){
            var newId = id;
            alert(newId);
            //  id here is always the first value from deleteSlip, and never changes
            if (confirmed)
            {
              // do stuff 
            }
        }); }

answered Feb 27 '13 at 16:45

hop

2,518
11
40
56

1

No, functions in JavaScript create a closure, so the callback carries with it its enclosing variable scope, including the `id` parameter. To do what you want, the `id` would need to be passed to `confirm`, and then `confirm` would need to pass it to the callback. But all that isn't necessary. – the system Feb 27 '13 at 16:47
@thesystem Buy your argument. But, won't passing it as parameter solve the problem? Was this solution tried? – hop Feb 27 '13 at 17:22
No, the problem is that the callback function is permanently bound to the dialogue via a new function created inside the `window.confirm` function that references it. By passing the `id` as well, there will still be the same problem. The new function will now reference the original function *and* the original ID. Also, the `confirm` function should be generic. having it receive and pass the `id`, makes it specific to the situation where you want to do that. Using closures lets your callback function reference whatever variables it wants without having to pass any when the callback is invoked. – the system Feb 27 '13 at 17:27

Value inside of callback is not being updated on each function call

2 Answers2