1

I've read Deleting array elements in JavaScript - delete vs splice (among other things) which has helped, but I am still at a loss as to why the below code works in the way it does.

There are 10 divs with the class "names" on the page. All currently with an opacity of 1 (included for context).

I'm trying to randomly choose 5 of them, and change their background colour to "red". After using splice() I would expect the array to decrease in size by 1 but this does not happen.

See live example here: http://jsfiddle.net/RussellEveleigh/Fr85B/1/ 

var z = document.getElementById("selected");

var r = function () {
b = document.getElementsByClassName("names");
c = [];
d = [];

    for (i = 0; i < b.length; i++) {
        if (window.getComputedStyle(b[i]).opacity == 1) {
            c.push(b[i]);
            }
        }

    for (i = 0; i < 5; i++) {
        num = Math.floor(Math.random() * c.length);
        z.innerHTML += c.length; // would expect "109876"
        d.push(c[num]);
        c.splice(num, num);

        }

    for (i = 0; i < d.length; i++) {
        d[i].style.backgroundColor = "red";
        }
    };

r();

Any help appreciated.

Community
  • 1
  • 1
Russell
  • 655
  • 2
  • 9
  • 21
  • 2
    `z.innerHTML` is a string, so the `+=` will do concatenation, not addition. So I think you'd want `z.innerHTML = (+z.innerHTML || 0) + c.length;`. **EDIT:** Actually, re-reading what your actual point of the code was, I'm not sure you want this. I think you meant to output the lengths as string, not a combined total – Ian Aug 02 '13 at 20:45
  • And not that it matters for this example, but you should always declare your variables with `var`. So that includes `b`, `c`, `d`, `i`, and `num` – Ian Aug 02 '13 at 20:48
  • @Ian I thought it was ok not to within a function? – Russell Aug 02 '13 at 21:00
  • 2
    Omitting `var` will declare them globally, which can definitely conflict with other code. And in ES5 strict mode, it will throw an exception if you try to set a variable defined without `var` – Ian Aug 02 '13 at 21:01
  • @Ian thanks for that you have saved me lots of future hassle! – Russell Aug 02 '13 at 21:08

1 Answers1

4

You should be using c.splice(num, 1);, because you want to remove one element from the array at a time, not num elements. This updated fiddle logs the length after each removal, and always outputs 9,8,7,6,5.

bfavaretto
  • 71,580
  • 16
  • 111
  • 150