-1

I developed an app using a constructor/prototypes approach to build and manage few objects. Using JavaScript and jQuery, one of the prototype creates input fields on which an anonymous function is declared on the onclick event.

Inside the anonymous function, I can easily access the primary object with an outside reference (var that = this) and I'm also able to access the properties linked to the input object itself. However, my input field is built within a for() loop and I also need to retain the reccursive variable iterated by the loop (the famous "var i" we use to see).

Since the "i" value is passed as a reference, every input will always retain the last value "i" got. So I found a workaround by attributing to the input object its own "i" property.

So my question is : is this a good approach ? Or should I use another way to get this work ?

Here's the code :

// Let's create the constructor
function familly(motherName,fatherName,children) {
    // We give to the properties their values
    this.motherName = motherName;
    this.fatherName = fatherName;
    this.children = children;
}

// Then we create a prototype that creates an input field for each child
familly.prototype.createChildrenInputs = function() {

    // I declare a variable that will serve as a reference to the main object (some people would name it "that")
    var famillyObject = this;

    // We pass into a loop all the children existing in the object property
    var children = this.children;
    for(var i in children) {

        // We create the input field
        var input = document.createElement('input');
            $(input).val(i);

            // !!! WORKAROUND !!! : I attach the "i" variable as a property to the input
            input.i = i;

            $(input).on('click', function() {

                // 1. The main object is accessible through the var famillyObject
                console.log(famillyObject);

                // 2. The value of the element is accessible with this.value
                console.log(this.value);


                // !!! HOWEVER !!!
                // ---------------

                // 3. The var "i" will always return "2"
                console.log(i);
                // 4. But the var "this.i" will show the good value (0, 1 or 2), since the reference was turned into a value with the workaround
                console.log(this.i);

            });
        document.body.appendChild(input);
    }
}

var mother = 'Nancy';
var father = 'Eric';
var children = [
    {
        'name':'Stephan',
        'gender':'male'
    },
    {
        'name':'Lois',
        'gender':'female'
    },
    {
        'name':'Andrew',
        'gender':'male'
    }
];

var newFamilly = new familly(mother,father,children);
newFamilly.createChildrenInputs();
  • use bind(), a function wrap, [].map instead of for, etc... – dandavis Dec 16 '14 at 01:23
  • I think Alexis' right, the link he put looks like an answer to me. However, I'm still wondering if my workaround is a good solution though. Thanks ! – Pierre-Luc Dec 16 '14 at 01:26
  • dandavis, those kind of one-line answer are not really helpful to me, since you'd need to contextualize a little how to use bind() for that specific case, without removing the "this.value" input parameter. – Pierre-Luc Dec 16 '14 at 01:27
  • What are you trying to achieve by not removing this.value? If you are just trying to create buttons, then why don't you use map() to map each object input to HTML button output? If you are trying to do something else, please revise your question with a description of that. Otherwise, what Alexis linked you to contains a pretty good solution. There are examples there. – akanevsky Dec 16 '14 at 01:42
  • 1
    i think it would be better not to invent your own solution to overcome the most-asked issue regarding for-loops in JS. it's not bad, but it's not great either; if html ever added an "i" property to elements, it would break. even with a better property name, i feel like there are broadly-understood paradigms for closing for-loop values, or avoiding the closure, and you lower readability by not embracing one. but it works! – dandavis Dec 16 '14 at 01:43
  • The more I read the possible duplicate and the more I find it doesn't answer to my specific case, since I'm not able to apply this fix to my input/event issue. Also, I still need to know if my woraround is a valid solution or a bad practice. – Pierre-Luc Dec 16 '14 at 01:44
  • 1
    you can replace the "for(var i in children)" with "[].map.call(this.children, function(child, i){" and not have to worry about stuff changing under you. (it gives you a "private i") – dandavis Dec 16 '14 at 01:47
  • @akanevsky : This is not a button, but an input field. Of course I'd like to use the solutions shown on the other article, but I just can't find a proper way to apply it to my case. – Pierre-Luc Dec 16 '14 at 01:48
  • @dandavis : Aleluia it works great that way, thanks a lot dear ! :-) – Pierre-Luc Dec 16 '14 at 01:51
  • @dandavis, that's a very elegant solution! Sorry to hijack the original question, but wouldn't it be shorter to just write this.children.map(function(child, i) { - or is there an advantage to writing [].map.call(this.children, function(child, i) { ? – akanevsky Dec 16 '14 at 02:05
  • this.children is not an array and thus has no map method. – dandavis Dec 16 '14 at 02:06
  • @dandavis, that's not true... it works the same way as your version, and in my understanding it's essentially the same, since you are substituting this.children in place of [] using the call() method. In any case, it works in jsFiddle. Maybe I'm missing something. – akanevsky Dec 16 '14 at 02:13
  • oops, i was thinking it pointed to element.children, you guys are right! sorry. – dandavis Dec 16 '14 at 03:08

1 Answers1

0

This topic is so well covered in SO but I'm providing an answer because some peripheral tidying is also possible ...

Instead of for() try looping through this.children with .forEach(). At each iteration, its function argument will form a closure trapping all local vars including i.

familly.prototype.createChildrenButtons = function() {
    var famillyObject = this;
    this.children.forEach(function(child, i) {
        var $input = $('<input />').val(i).on('click', function() {
            console.log(famillyObject);
            console.log(this.value);
            console.log(i);
        });
        $(document).append($input);
    });
};
Roamer-1888
  • 19,138
  • 5
  • 33
  • 44
  • Thanks Roamer, it seems to work. I have no doubt the topic is well covered, the problem is I wasn't able to find out something revelent with those popular keywords : for-loop, variables, scope, etc. Since I'm new to JavaScript and SO, it's also kinda hard to know all those precise terms and concepts to target the right thing. Anyways, thanks for your explanation ! – Pierre-Luc Dec 16 '14 at 02:09
  • @dandavis, from the question I understand that `this.children` is an array not a collection. If so, then it has a `.forEach()` method. No need to `.apply()`. And if `this.children` was a collection, then `this.children.each(...)` would be the way to go. – Roamer-1888 Dec 16 '14 at 02:12
  • @Pierre-Luc, agreed, SO is hard to penetrate. Good luck. – Roamer-1888 Dec 16 '14 at 02:14
  • yes, i mis-read the code and never clicked the fiddle, sorry. no need to make it more complicated. – dandavis Dec 16 '14 at 03:08
  • No worries @dandavis, I initially saw the word 'children' and thought the same. – Roamer-1888 Dec 16 '14 at 14:36