-1

I'm trying to splice an array when an element is clicked. When I console.log the new array it's irregular and sometimes removes the wrong array element and the last index will never be removed. Any one here with a good explanation to this?

var container = document.getElementById('container'),
notDone = document.getElementsByClassName('notDone'),
deleting = document.getElementsByClassName('delete'),
div,
remove,
i,
toDo = [
'One',
'Two',
'öäå'],


for(i = 0; i < toDo.length; i++){
    div = document.createElement('div');
    div.innerHTML = toDo[i];
    div.className = 'notDone';
    remove = document.createElement('div');
    remove.innerHTML = '<p>Remove</p>';
    remove.setAttribute("data-id", i);
    remove.className = 'delete';

    container.appendChild(div);
    div.appendChild(remove);

    notDone[i].addEventListener('click', function(){

        if(this.classList.contains('notDone')){
            this.className = 'done';
        }else{
            this.className = 'notDone';
        }
    });

    deleting[i].addEventListener('click', function(event){
        event.stopPropagation();
        var shouldDelete = this.getAttribute("data-id");
        console.log('va' + shouldDelete);
        toDo.splice(shouldDelete, 1);
        this.parentNode.remove();
        console.log(toDo);
    });
}
var check = document.getElementById('check');
check.addEventListener('click', function(){
    console.log(toDo + ' checking array')
});
frostman
  • 566
  • 2
  • 12
  • 25
NinjaGrisen
  • 29
  • 1
  • 6
  • The question is, why would `this.getAttribute("data-id")` return an array, and not a string ? – adeneo Apr 23 '15 at 21:55
  • @adeneo: `toDo` is the array, `shouldDelete` is an integer string. – Bergi Apr 23 '15 at 22:00
  • That is a good question that Im not able to answer, I'm new to JS and still learning. Any thoughts on how I should solve this problem? – NinjaGrisen Apr 23 '15 at 22:01
  • @Bergi - my eyesight is going .......... – adeneo Apr 23 '15 at 22:01
  • I guess it happens because you never update the original index, so after you remove the second element (data-id=1) the former third one (data-id=2) moves to the second position. If you click on it, it will report itself as having data-id=2, and you'll splice the current third element (data-id=3). – ffflabs Apr 23 '15 at 22:05

1 Answers1

0

it sometimes removes the wrong array element

The problem you are facing is that the data-id is storing a fixed index, yet the toDo array is mutable. When you remove an element from the array, all the indices behind it become invalid. For example when you .splice(0, 1), then you cannot do splice(2, 1) any more because the array has only two items left.

the last index will never be removed.

It would, if you clicked the last element at first.


What you can do instead is either

  • not removing the indices from the toDo array (and cutting the whole array), but just set the array values to null; or
  • not store the original indices, but the values that you want to remove, and search them (using indexOf) in the current array every time you remove an element
Bergi
  • 630,263
  • 148
  • 957
  • 1,375