Loading ...
Sorry, an error occurred while loading the content.

Re: Be careful when making functions in a loop...

Expand Messages
  • Douglas Crockford
    ... It is generally a bad idea to define a function in a loop. If the function refers to a variable that changes in the loop, then it will bind to the final
    Message 1 of 8 , Jul 25, 2008
    • 0 Attachment
      --- In jslint_com@yahoogroups.com, "tonyorlando" <rhettanderson@...>
      wrote:
      >
      > In 3000 lines of JS code, I'm completely clean according to JSLint,
      > except for five messages like this:
      >
      > "Be careful when making functions within a loop. Consider putting the
      > function in a closure."
      >
      > These are marked as "errors," but since the message is "be careful,"
      > they seem more like warnings.
      >
      > I'm not sure what to do about these.

      It is generally a bad idea to define a function in a loop. If the
      function refers to a variable that changes in the loop, then it will
      bind to the final state, not the current state, of the variable, which
      usually causes errors. The remedy is to move the function declaration
      out of the loop (which is also an optimization) or to wrap the
      function declaration inside another function that will bind the loop
      values.

      Since you didn't include an example from your loops, I can't tell you
      which you should do.
    • Ben Collver
      Hi Doug, I wrote an example script and it appears to bind to the current value of the variable i . The script is at the following URL.
      Message 2 of 8 , Aug 13, 2008
      • 0 Attachment
        Hi Doug,

        I wrote an example script and it appears to bind to the current value
        of the variable "i".  The script is at the following URL.
        http://nopaste.info/551fef77cf.html 

        Example output:
        0,a
        0,b
        0,c
        1,a
        1,b
        1,c
        2,a
        2,b
        2,c
        3,a
        3,b
        3,c

        If you please would you show an example of the problem?

        Thank you,

        Ben
      • Douglas Crockford
        ... Create a page with three buttons, a, b, and c. When you click on a button, it alerts its number. Modify your loop to get each element by id and attach an
        Message 3 of 8 , Aug 13, 2008
        • 0 Attachment
          --- In jslint_com@yahoogroups.com, "Ben Collver" <tylx@...> wrote:
          > I wrote an example script and it appears to bind to the current value
          > of the variable "i". The script is at the following URL.

          > If you please would you show an example of the problem?

          Create a page with three buttons, a, b, and c. When you click on a
          button, it alerts its number. Modify your loop to get each element by
          id and attach an onclick handler that alerts the index. If you do it
          incorrectly, they will all alert the same wrong number.
        • Ben Collver
          I created a page with three buttons, a, b, and c. When you click on a button, it alerts its number. I modified the loop to attach an onclick handler to each
          Message 4 of 8 , Aug 19, 2008
          • 0 Attachment
            I created a page with three buttons, a, b, and c. When you click on a button, it alerts its number. I modified the loop to attach an onclick handler to each input element and alert the index. They all alert 4 as expected.

            I must have misunderstood the instructions. The page is at http://bencollver.googlepages.com/looptest.html

            Ben


            ________________________________________________________________________
            Posted by: "Douglas Crockford" douglas@... douglascrockford
            Date: Wed Aug 13, 2008 11:31 am ((PDT))

            Create a page with three buttons, a, b, and c. When you click on a
            button, it alerts its number. Modify your loop to get each element by
            id and attach an onclick handler that alerts the index. If you do it
            incorrectly, they will all alert the same wrong number.
          • William Chapman
            Ben, Another approach: function main() { var arr = [ a , b , c ]; var closedValue; for (var i = 0; i
            Message 5 of 8 , Aug 20, 2008
            • 0 Attachment
              Ben,

              Another approach:

              function main() {
              var arr = ['a', 'b', 'c'];
              var closedValue;
              for (var i = 0; i < 3; i++) {
              closedValue = i;
              document.getElementById(arr[i]).onclick = function(closedValue) {
              alert(closedValue);
              });
              }
              }

              In your code, this line:
              alert(i)
              displays the value of i **when the button is clicked**, as opposed to,
              **when the function is assigned to onclick.**

              For me, the original point - that one needs to take care when defining
              functions in a loop - is valid. Which isn't the same as saying it's never a
              good idea.

              -- Bill

              On Tue, Aug 19, 2008 at 1:40 PM, Ben Collver <tylx@...> wrote:

              > I created a page with three buttons, a, b, and c. When you click on a
              > button, it alerts its number. I modified the loop to attach an onclick
              > handler to each input element and alert the index. They all alert 4 as
              > expected.
              >
              > I must have misunderstood the instructions. The page is at
              > http://bencollver.googlepages.com/looptest.html
              >
              > Ben
              >
              > __________________________________________________________
              > Posted by: "Douglas Crockford" douglas@...<douglas%40crockford.com>douglascrockford
              > Date: Wed Aug 13, 2008 11:31 am ((PDT))
              >
              >
              > Create a page with three buttons, a, b, and c. When you click on a
              > button, it alerts its number. Modify your loop to get each element by
              > id and attach an onclick handler that alerts the index. If you do it
              > incorrectly, they will all alert the same wrong number.
              >
              >
              >


              [Non-text portions of this message have been removed]
            • William Chapman
              Ben, Please forgive my previous email which is totally wrong! The following actually works (shamelessly copied from Crockford: The Good Stuff page 39):
              Message 6 of 8 , Aug 20, 2008
              • 0 Attachment
                Ben,

                Please forgive my previous email which is totally wrong! The following
                actually works (shamelessly copied from Crockford: "The Good Stuff" page
                39):

                function main() {
                var arr = ['a', 'b', 'c'];
                for (var i = 0; i < 3; i++) {
                document.getElementById(arr[i]).onclick = function(i) {
                return function (event) {
                alert(i);
                };
                }(i);
                }
                }

                Other comments:
                (1) The onclick handlers in your HTML <input > elements are overwritten by
                main() above, so are not needed..
                (2) Your original forEach() function is not used in the above approach.

                Sorry about the bum steer!

                -- Bill

                On Wed, Aug 20, 2008 at 9:24 AM, William Chapman <jeddahbill@...>wrote:

                > Ben,
                >
                > Another approach:
                >
                > function main() {
                > var arr = ['a', 'b', 'c'];
                > var closedValue;
                > for (var i = 0; i < 3; i++) {
                > closedValue = i;
                > document.getElementById(arr[i]).onclick = function(closedValue) {
                > alert(closedValue);
                > });
                > }
                > }
                >
                > In your code, this line:
                > alert(i)
                > displays the value of i **when the button is clicked**, as opposed to,
                > **when the function is assigned to onclick.**
                >
                > For me, the original point - that one needs to take care when defining
                > functions in a loop - is valid. Which isn't the same as saying it's never a
                > good idea.
                >
                > -- Bill
                >
                >
                > On Tue, Aug 19, 2008 at 1:40 PM, Ben Collver <tylx@...> wrote:
                >
                >> I created a page with three buttons, a, b, and c. When you click on a
                >> button, it alerts its number. I modified the loop to attach an onclick
                >> handler to each input element and alert the index. They all alert 4 as
                >> expected.
                >>
                >> I must have misunderstood the instructions. The page is at
                >> http://bencollver.googlepages.com/looptest.html
                >>
                >> Ben
                >>
                >> __________________________________________________________
                >> Posted by: "Douglas Crockford" douglas@...<douglas%40crockford.com>douglascrockford
                >> Date: Wed Aug 13, 2008 11:31 am ((PDT))
                >>
                >>
                >> Create a page with three buttons, a, b, and c. When you click on a
                >> button, it alerts its number. Modify your loop to get each element by
                >> id and attach an onclick handler that alerts the index. If you do it
                >> incorrectly, they will all alert the same wrong number.
                >>
                >>
                >>
                >
                >


                [Non-text portions of this message have been removed]
              • Ben Collver
                Bill, Thank you for the response. I worked backward from it to get to an example of the problem. It behaves the same on several hosts: it outputs three 3 s.
                Message 7 of 8 , Aug 25, 2008
                • 0 Attachment
                  Bill,

                  Thank you for the response. I worked backward from it to get to an example of the problem. It behaves the same on several hosts: it outputs three 3's. My original WSH example must have avoided the problem.

                  http://bencollver.googlepages.com/looptest3.html

                  Cheers,

                  Ben
                Your message has been successfully submitted and would be delivered to recipients shortly.