Javascript deleting wrong elements from the DOM

Question

I have the code over at Paste Bin @ http://pastebin.com/bu2sz2C0. I have been given this code as an academic assignment and is not code i have written myself with the exception of the killFrog function.

The issue I am having is with the killFrog function. Each frog is stored in an Array called Frogs and then the DOM elements representing these frogs are stored as _frogs. I am trying to make it so that when you click the Kill Frog button the killFrog function is triggered and deletes a random frog from the Array, and then removes that Frog from the DOM.

Using console.log() I have found the ID of the frog and the attribute "data-frogid" of the frog and is outputting correctly which of the frogs should be removed from the DOM however, sometimes it removes the correct frog from the dom, sometimes the wrong frog or sometimes not at all. I am baffled by this as it is outputting to the console which frog should be removed but then not removing the correct one.

Here is the code I have written for killFrog

killFrog = function(){
          console.log('////////////Selected Frog to Die///////////////');
          var _randomFrog = this.Frogs[Math.floor(Math.random()*this.Frogs.length)];
          console.log(_randomFrog);
          var indexOfFrog = this.Frogs.indexOf(_randomFrog);
          this.Frogs.splice(indexOfFrog,1); //Remove frog from array
          console.log(_frogs.childNodes);
          console.log("ID of FROG to be deleted" + _randomFrog.getId());
          for (var i = 0; i < _frogs.childNodes.length; i++) {
             if (_randomFrog.getId() == _frogs.childNodes[i].getAttribute("data-frogid")){
                console.log("ID : " + _randomFrog.getId() +  " Data attribute : " + _frogs.childNodes[i].getAttribute("data-frogid") + " NAME: " + _randomFrog.name + " DOM eleement name : " + _frogs.childNodes[i].innerHTML + " To be removed");
                // _frogs.removeChild(_frogs.childNodes[i]);
                _frogs.childNodes[i].parentNode.removeChild(_frogs.childNodes[i]);
             }
          };
          console.log(this.Frogs.length + "Frogs left in the array");
      },

Update:

Code updated to remove duplicate i variable @ http://pastebin.com/4JiQBK12

I believe this is due to the way the Id's are set on the array of Frogs. From the code i have updated, when I display the data attribute of the frog it does not correspond with the ID that it gets from the Array.

could this be the way that the id's are set for the frogs at

frog.setId(this.Frogs[this.Frogs.length - 1] ? this.Frogs[this.Frogs.length - 1].getId() + 1 : 1);

Answer 1

Because you're using the revealing pattern to set your prototype, all instances of Frog (and Pond) are using the same data. For example, if you set the id of frog 1 to 5, frog 2's id will also be 5. Here's a simplified example of what you have:

function foo () {}
foo.prototype = function () {
    var _id = 5;
    return {
        getId: function() {
            return _id;
        },
        setId: function(id) {
            _id = id
        }
    }
}();
var foo1 = new foo();
var foo2 = new foo();
foo1.setId(1);
console.log(foo2.getId()); //1
foo2.setId(8);
console.log(foo1.getId()); //8

To solve this, don't use the revealing pattern to define the prototype. There are several ways, what i've shown below is just one of them.

function foo () {
    var _id = 0;
    this.getId = function() {
        return _id;
    };
    this.setId = function(id) {
        _id = id
    };
}
var foo1 = new foo();
var foo2 = new foo();
foo1.setId(1);
console.log(foo2.getId()); //0
foo2.setId(8);
console.log(foo1.getId()); //1

Answer 2

You're declaring the i variable twice in the same scope:

var i = this.Frogs.indexOf(_randomFrog);

and

for (var i = 0; i < _frogs.childNodes.length; i++) {

Variables should always be declared at the top of the scope. Hoisting and overwriting can cause a lot of confusion, like this.

For this situation, the vars should not be named the same.