Why does on click if checked malfunction after dealing with 2nd+ index in array

Question

So i got this working with the first value x to be pushed and spliced but after it only pushes or doesn't do anything

for(x in aL.results){$('#tblListings').append(
            '<tr>'
                +'<td> 
    <input type="checkbox" name="updateListings[]" value='+x+' onclick="doSomething(this)">
</td>+'</tr>'
        );}

function doSomething(cb) {
    if ($(cb).is(":checked")) {
        uLIndex.push(cb.value);
        alert(uLIndex);
    } else {
        uLIndex.splice(cb.value, 1);
        alert(uLIndex);
    }       
}

`cb.value` is not an index in the array, it's a string ("`+x+`"). See [`splice`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice) — Teemu, Jan 04 '17 at 16:47
Tested onchange event still same result, checking checkbox 2 and unchecking and re checking rsult is 1,1 — Andrei Pak, Jan 04 '17 at 16:50
Please re-read my comment, and also the documentation I've linked. — Teemu, Jan 04 '17 at 16:50
cb.value is a string representation of the index of the listing — Andrei Pak, Jan 04 '17 at 16:51
@Teemu it can... if that part is replaced with a number by whatever backend is in use — Kevin B, Jan 04 '17 at 16:52
got to admit i like the responces of stack overflow, been just googling my problem. — Andrei Pak, Jan 04 '17 at 16:54
@KevinB Doesn't look to be replaced in the example though. I could imagine there being more these checkboxes, it doesn't make much sense to hardcode indices to the values of the inputs. Most likely user won't check and re-check them in a specific order. — Teemu, Jan 04 '17 at 16:55
ok i hope this makes more sense even though i got a good answer as a solution — Andrei Pak, Jan 04 '17 at 17:00

Rory McCrossan · Accepted Answer · 2017-01-04T16:58:15.210

2

Your issue is because you're providing the string value of the checkbox to slice(). Instead you need to provide the numerical index of the item to remove in the array. You can use indexOf() to retrieve that:

var uLIndex = [];

function doSomething(cb) {
    if ($(cb).is(":checked")) {
        uLIndex.push(cb.value);
        console.log(uLIndex);
    } else {
        uLIndex.splice(uLIndex.indexOf(cb.value), 1); // note the change here
        console.log(uLIndex);
    }       
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<input type="checkbox" name="updateListings[]" value="1" onclick="doSomething(this)">
<input type="checkbox" name="updateListings[]" value="2" onclick="doSomething(this)">
<input type="checkbox" name="updateListings[]" value="3" onclick="doSomething(this)">
<input type="checkbox" name="updateListings[]" value="4" onclick="doSomething(this)">

Also note that it's considered better practice to use unobtrusive event handlers over the now outdated on* event attributes. You can also simplify the code to build the array on each click using jQuery's map() method, instead of having to track additions/removals manually. Try this:

$('input:checkbox').change(function() {
  var uLIndex = $('input:checkbox:checked').map(function() {
    return this.value;
  }).get();
  console.log(uLIndex);
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<input type="checkbox" name="updateListings[]" value="1">
<input type="checkbox" name="updateListings[]" value="2">
<input type="checkbox" name="updateListings[]" value="3">
<input type="checkbox" name="updateListings[]" value="4">

As with most things that involve jQuery, this version has the advantage of using much simpler logic but being slightly slower (although we're talking milliseconds only). If you need a higher level of performance I'd argue that you probably shouldn't use jQuery at all.

edited Jan 04 '17 at 16:58

answered Jan 04 '17 at 16:52

Rory McCrossan

331,213
40
305
339

Although in your second solution, i am concerned is there is 2 different checkbox arrays for 2 different functions, how can java-script be differentiated... – Andrei Pak Jan 04 '17 at 17:06
I'm not sure what you mean - it still builds the same single array. The logic of both examples has the same result. – Rory McCrossan Jan 04 '17 at 17:07
I am just wondering if there is 2 different sets of check boxes, one for listings and the other lets say to edit only title, or to edit title and descriptions or even any possibilities. so it works with set listings, but how can it see different when looking at set properties – Andrei Pak Jan 04 '17 at 17:11
Ah I see - in that case you would need to restrict the `map()` function to only select those checkboxes you require. I'd suggest starting a new question with your full HTML sample. – Rory McCrossan Jan 04 '17 at 17:14
I may, but for now I am satisfied with your solution, the only issue is the second solution $('input:checkbox').change(function() { doesnt seem to catch any changes – Andrei Pak Jan 04 '17 at 17:18
ok started a new question http://stackoverflow.com/questions/41470887/no-return-oninputcheckbox-changefunction – Andrei Pak Jan 04 '17 at 18:32
the second solution seams to only work on pure html input but not appended, How could i use it. – Andrei Pak Jan 04 '17 at 20:31
For appended content you need to use a delegated event handler: http://stackoverflow.com/questions/203198/event-binding-on-dynamically-created-elements – Rory McCrossan Jan 04 '17 at 21:24
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/132341/discussion-between-andrei-pak-and-rory-mccrossan). – Andrei Pak Jan 04 '17 at 22:24
I figured it out, original data was coded in ajax, so i my mistake was makign the event handler outside the scope – Andrei Pak Jan 05 '17 at 00:16

Why does on click if checked malfunction after dealing with 2nd+ index in array

1 Answers1