jquery each loop only looping once and if using else code stops

Question

I've got two problems with the following javascript and jquery code. The jquery each loop only iterates once, it gets the first element with the right ID does what it needs to do and stops.

The second problems is that when I use the else in the code the one inside the each function, it doesn't even tries the next if, it just exits there. I'm probably doing something fundamental wrong, but from the jquery each function and what I'd expect from an else, I don't see it.

Javascript code:

var $checkVal;
var $checkFailed;

$("#compliance").live("keypress", function (e) {

 if (e.which == 10 || e.which == 13) {
    var checkID = $(this).parents('td').next().attr('id');
    var checkVal = $(this).val();
    $('#' + checkID).each(function () {
        var cellVal = $(this).text();
        if (checkVal == cellVal) {
            $(this).removeClass("compFail").addClass("compOk");
        } else {
            $(this).removeClass("compOk").addClass("compFail");
            var checkFailed = True;
        }
    });
    if (checkFailed == 'True') {
        (this).addClass("compFail");
    } else {
        (this).addClass("compOk");
    }
 }
});

How could I get the each loop to iterate through all instances of each element with the id assigned to the variable checkID, and get the code to continue after the else, so it can do the last if?

You shouldn't have more than one instance of an ID on a page. Or did I misunderstand that. — Jeff Shaver
– Jeff Shaver, Commented Mar 12, 2013 at 12:32
$('#' + checkID) will only return a single element, since you are using the ID-selector and an ID is meant to be unique to a single element. Therefor running .each() on the returned set of elements is pointless, since it will always be a single element. — Christofer Eliasson
– Christofer Eliasson, Commented Mar 12, 2013 at 12:33
Would need to see some html for your first problem. For problem two, notice that you declare a variable inside the .each, and then attempt to use it outside. — Mike C.
– Mike C., Commented Mar 12, 2013 at 12:35
Mike C, for the second problem, I declare the variable in the second line, outside all the functions. Should it work like that as the other variable does? — isaapm
– isaapm, Commented Mar 12, 2013 at 13:28

benzonico · Accepted Answer · 2013-03-12 12:34:10Z

4

An id should appear on a page only once. If you want to have multiple elements with same id, then use a class, not an id.

Your each loop iter only once because you are selecting by id thus you are selecting only one element in the page. If you change you elements to a class it should work like you expect.

answered Mar 12, 2013 at 12:34

benzonico

10.9k5 gold badges46 silver badges51 bronze badges

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

Comments

Mike C. · Accepted Answer · 2013-03-12 13:42:59Z

This is to illustrate what I'm talking about in my comment, so that you do not remove the wrong var:

var checkVal;
var checkFailed;

$("#compliance").live("keypress", function (e) {

 if (e.which == 10 || e.which == 13) {
    var checkID = $(this).parents('td').next().attr('id');
    //HERE is the first edit
    checkVal = $(this).val();
    $('#' + checkID).each(function () {
        var cellVal = $(this).text();
        if (checkVal == cellVal) {
            $(this).removeClass("compFail").addClass("compOk");
        } else {
            $(this).removeClass("compOk").addClass("compFail");
            //HERE is the second
            checkFailed = True;
        }
    });
    if (checkFailed == 'True') {
        (this).addClass("compFail");
    } else {
        (this).addClass("compOk");
    }
 }
});

Normally, the way you have it would cause a compile-time error (in a typed language like C#) for redeclaring a variable. Here, it's not clear to me if it will be used as a local variable (ignoring your global variable) or if javascript will combine them and consider them the same. Either way, you should use it as I have shown so that your intent is more clear.

EDIT: I have removed the $ from your variables (var $checkVal) as on jsFiddle it was causing issues. SO if you do not need those $'s, then remove them. Also, note that testing on jsFiddle indicates that you do not need to change your code (other than possibly removing the $ from your declaration) as javascript appears to consider them the same variable, despite the redeclaration, which I find a bit suprising tbh.

Hi Mike,thanks a lot for the explanation, I understand it now. I've just declared the variables twice instead of declaring it once and assigning a value later.

Jon Cram · Accepted Answer · 2013-03-12 12:34:24Z

1

The jquery each loop only iterates once, it gets the first element with the right ID does what it needs to do and stops.

Yes, this is absolutely right for the code you're using:

$('#' + checkID).each(function(){};)

ID attributes are unique. There must be only one element with a given ID in the DOM. Your selector can match only one element. You are iterating over a collection containing just 1 item.

answered Mar 12, 2013 at 12:34

Jon Cram

17.5k24 gold badges81 silver badges110 bronze badges

Collectives™ on Stack Overflow

jquery each loop only looping once and if using else code stops

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related