Self-recursive calls (probably, due to closure involving)

Question

I have basically this code:

$(document).ready(function() {
  (function fillOutTable() {
    $.ajax({
      type: 'GET',
      url: 'someUrl',
      success: function(data) {
        var myTable = $("#tbl1");
        for(var i in data.myCollection) {
          /*filling out the table*/
          var str = $(
            ["<tr>",
            "<td>", counter++, "</td>",
            "<td>",
                "<a href='#' class='myClass'>", data.myCollection[i].object1.name, "</a>",
            "</td>",
            "<td>", title1,  "</td>",
            "<td>", title2,  "</td>",
            "<td>", title3,  "</td>"
            "</tr>"].join("")
          );

          myTable.find("tbody").append(str);
          myTable.find("tbody").on("click", ".myClass", function() {
            $('#twitterBtstrModal').on('shown.bs.modal', function() {
              alert('yes: ' + data.myCollection[i].object1.name);
            }).modal();
          });
        }
      },

      error: function(jqXHR, textStatus, errorThrown) {
        showNoDataMessage();
        alert('Something went wrong!');
      }
    });
})();

There are 2 issues with this code:

When I click on a link (which is the literal for data.myCollection[i].object1.name) in the table, it always showms me the same data yes: data.myCollection[i].object1.name in the alert, no matter what link I click. Although the html code in different for all of them.
The more I click on the links, the more alerts I get per click: the first time I there is only one alert appears, the next time - 2 ones, the next time - 4 or something.

see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Closures#Creating_closures_in_loops.3A_A_common_mistake — Arun P Johny, Mar 28 '14 at 02:51
and http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example — Arun P Johny, Mar 28 '14 at 02:54

Arun P Johny · Accepted Answer · 2014-03-28T03:15:18.907

1

There are multiple problems

usage of a closure variable in a loop
binding event handler inside another one... etc

Try

jQuery(function ($) {
    var myTable = $("#tbl1"),
        $tbody = myTable.find("tbody"),
        $model = $('#twitterBtstrModal');

    $.ajax({
        type: 'GET',
        url: 'someUrl',
        success: function (data) {

            $.each(data.myCollection, function (i, data) {
                /*filling out the table*/
                var str = $(
                ["<tr>",
                    "<td>", counter++, "</td>",
                    "<td>",
                    "<a href='#' class='myClass'>", data.myCollection[i].object1.name, "</a>",
                    "</td>",
                    "<td>", title1, "</td>",
                    "<td>", title2, "</td>",
                    "<td>", title3, "</td>"
                "</tr>"].join(""));

                //store the row data using data api
                $(str).appendTo($tbody).data('rowdata', data)
            });
        },

        error: function (jqXHR, textStatus, errorThrown) {
            showNoDataMessage();
            alert('Something went wrong!');
        }
    });

    //register these events only once

    //a delegated handler which will get the current rows data and set it to the modal
    $tbody.on("click", ".apiKeyName", function () {
        $model.data('mydata', $(this).closest('tr').data('rowdata')).modal();
    });
    $model.on('shown.bs.modal', function () {
        //read the data from the modal's data 
        var data = $model.data('mydata');
        alert('yes: ' + data.object1.name);
    })
});

_{Disclaimer: Not tested}

edited Mar 28 '14 at 03:15

answered Mar 28 '14 at 02:59

Arun P Johny

384,651
66
527
531

`alert('yes: ' + data.object1.name);` - you forgot about what I have in my code `alert('yes: ' + data.myCollection[i].object1.name)`; – Incerteza Mar 28 '14 at 03:04
@Alex I change changed the way the `data` is stored... so I don't have to access the collection any more to access the rows data... look at the lines `var data = $model.data('mydata');`, `$model.data('mydata', $(this).closest('tr').data('rowdata'))` and `$(str).appendTo($tbody).data('rowdata', data)` – Arun P Johny Mar 28 '14 at 03:05
thanks, but that's not what I need. I don't want to improve the code + fixing the errors, I want to fix the errors. As improving makes it hard to follow, to see the difference. – Incerteza Mar 28 '14 at 03:11
@Alex then I'm not your guy... fixing code involves improving the mistakes in it... – Arun P Johny Mar 28 '14 at 03:12
@Alex it will be harder/impossible to fix your code without a redesign because the from my point of view the design itself has issues... in your case it will involve unbinding event handlers creating a local closure etc thus creating a lot more mess that really required – Arun P Johny Mar 28 '14 at 03:13
it's for education purpose, not for production. – Incerteza Mar 28 '14 at 03:15

Self-recursive calls (probably, due to closure involving)

1 Answers1