1

I have this code:

document.getElementById('img'+i).onclick = function(){popup_show('popup',array_msg[i]+'|||'+date('Y-m-d',strtotime(lec_date))+'-==-'+str_view_forConflict, 'AddEditSchedule;;popup_drag2;;EditSched;;'+String(array_msg_id[3])+';;view', 'popup_drag', 'popup_exit', 'screen-center', 0, 0);};

...but when I click on the image, the data of array_msg[i] is the last data of the loop, meaning the index is the length of the loop. I use IE for this.

Please give me an idea on how to do this. In FF, it works fine because I use setAttribute.

@bobince

document.getElementById('img'+i).onclick= popup_show.bind(window, 'popup',array_msg[i]+'|||'+date('Y-m-d',strtotime(lec_date))+'-==-'+str_view_forConflict,'AddEditSchedule;;popup_drag2;;EditSched;;'+array_msg_id[3]+';;view','popup_drag', 'popup_exit', 'screen-center', 0, 0    ); 
                if (!('bind' in Function.prototype)) {
                    Function.prototype.bind= function(owner) {
                        var that= this;
                        var args= Array.prototype.slice.call(arguments, 1);
                        return function() {
                            return that.apply(owner,
                                args.length===0? arguments : arguments.length===0? args :
                                args.concat(Array.prototype.slice.call(arguments, 0))
                            );
                        };
                    };
                }
Improve this question

5 Answers 5

Reset to default
2

Treby, this is cleaned up version of your answer. The additional anonymous anonymous function that you added isn't necessary.

for (var i = 0, l = array.length; i < l; i++ ) {
  document.getElementById(i + '05').onclick = (function(tmp) {
    return function() { 
      popup_show(
        "popup", 
        array_msg[tmp] + '|||' + date('Y-m-d', strtotime(lec_date)) + '-==-' + str_view_forConflict, 
        "AddEditSchedule;;popup_drag2;;EditSched;;" + String(array_msg_id[3]) + ";;view", 
        "popup_drag", "popup_exit", "screen-center", 0, 0
      );
    };
  })(i);
}

Edited to fix closure issue

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

Comments

2

You've hit the loop-variable-closure problem. This is a very common gotcha in C-style languages with closures, such as JavaScript and Python. See the accepted answer of this question for a solution involving binding the loop variables in a second closure.

A slightly less nested solution is to use function.bind():

for (var i= 0; i<something.length; i++) {
    document.getElementById('img'+i).onclick= popup_show.bind(window, 'popup',
        array_msg[i]+'|||'+date('Y-m-d',strtotime(lec_date))+'-==-'+str_view_forConflict,
        'AddEditSchedule;;popup_drag2;;EditSched;;'+array_msg_id[3]+';;view',
        'popup_drag', 'popup_exit', 'screen-center', 0, 0
    );
}

however since this method is an ECMAScript Fifth Edition feature not supported by most browsers yet it needs a little help — see the bottom of this answer for a backwards-compatible implementation.

Improve this answer

4 Comments

i have an error error details Message: 'null' is null or not an object Line: 261 Char: 3 Code: 0 line 261 pertains to document.getElementById(;popup_drag;)['target'] = id;
Impossible to say without the code of that function really, but it's looking for an element with a given ID and not finding it. I don't know what the semicolons are doing there; if there were really like that in your source code that would be a syntax error and wouldn't run at all.
check out my question: this what i did
I mean that function where it is going wrong — the error is occurring inside another function, presumably popup_show. Anyway, you will need the code that defines function.bind to go before the loop that uses it.
2

You need to use a closure. It would help if you provided the loop code as well as the code that is executed in the loop, but assuming you have a standard for loop iterating through an array, the following code should work:

for (var i = 0, l = array.length; i < l; i++)
{
    (function(i) {
        document.getElementById("img" + i).addEventListener("click", function() {
            popup_show("popup", array_msg[i] + "|||" + date("Y-m-d", strtotime(lec_date)) + "-==-" + str_view_forConflict, "AddEditSchedule;;popup_drag2;;EditSched;;" + String(array_msg_id[3]) + ";;view", "popup_drag", "popup_exit", "screen-center", 0, 0);
        }, false);
    })(i);
}

Also, you shouldn't be using setAttribute in Firefox. Instead, use element.onclick or, preferably, element.addEventListener, which allows you to add multiple functions to be called when an event fires and thus this plays nicely with other code (if two bits of code assign a function to, say, a click event in the form element.onclick = function() { ..., then the second assignment overrides the first—not good). I've used element.addEventListener in my code example above.

Improve this answer

9 Comments

what do you suggest to use then? .onclick??
I would definitely recommend using onclick rather than setAttribute, but these days, onclick is also a bit dated. I would recommend using addEventListener instead. More information about addEventListener and its IE equivalent (attachEvent) can be found here: developer.mozilla.org/en/DOM/element.addEventListener.
@Treby: Which browser are you testing it in?
@Treby: It is a performance best-practice. It's very inefficient to constantly look up the length property of an array every loop iteration, so we cache the length of the array in a variable, l, and use this whenever we need to access the array's length during the loop.
@Treby: The above code doesn't take IE into account at all—try testing it in Safari, Chrome, Firefox, or another standards-compliant browser...
|
1

Closures. You need to use JavaScript Closures. See if answers to this question help.

Improve this answer

Comments

-1

Working Answer:

var closures = [];
for (var i = 0; i < array.length; i++){
  closures[i] = (function(tmp) {
        return function() {
        document.getElementById(tmp + '05').onclick = function(){popup_show("popup", array_msg[tmp]+'|||'+date('Y-m-d',strtotime(lec_date))+'-==-'+str_view_forConflict, "AddEditSchedule;;popup_drag2;;EditSched;;"+ String(array_msg_id[3]) +";;view", "popup_drag", "popup_exit", "screen-center", 0, 0)};
        };
})(i);

  closures[i]();
}

Thanks to Steve Harrison Answer. I got an idea to wrap around it

Improve this answer

1 Comment

If the question is answered, mark it as such. A note on the code: there's likely not a reason to save references to each closure. See my answer for a revised version of what you have.

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.