JavaScript splice function inside foreach loop decrements index

You splice elements from the array, which you iterated, therefore indexes in "todos" reduced. Sorry for my bad english.

var notDonedTodos = [];
angular.forEach($scope.todos, function(todo, i)
{
    if(!todo.done)
    {
       notDonedTodos.push(todo);
    }
});

$scope.todos = notDonedTodos;

This is happening because forEach only knows about the initial state of the array, and therefore calls your method twice, even if the first call removes an item from the array. Just do a simple while loop instead:

var i = $scope.todos.length;
while (i--){
    if ($scope.todos[i].done){
        $scope.todos.splice(i, 1);
    }
}

The issue with the each iteration is that it removes an item from the array causing an iteration to be skipped. jQuery has a nice grep method that returns all elements matching a certain criteria that is determined by a provided anonymous function.

var todos =[{id:1, done:false},{id:2, done:true},{id:3, done:true}];

function removeCompleted(todos){
    return $.grep(todos,function(todo){
        return todo.done == false;
    });
}

todos = removeCompleted(todos);
console.log(todos);

Working Example http://jsfiddle.net/ktCEN/

Documentation

The alternative i've found myself is to use the array.filter method. It is the most easy way to filter an array based on object keys. If you're working on an IE8 project (poor you) you'll need to add a polyfill for this function since it's fairly new to JavaScript.

Everything you need to know about javascript.

Answer code:

$scope.clearCompleted = function() {
    $scope.todos = $scope.todos.filter(function(item) {
        return !item.done;
    });
}

As another alternative, you could just decrement your index each time you do a splice. For instance:

$scope.clearCompleted = function() {
    angular.forEach($scope.todos, function(todo, i) {
        if(todo.done) {
            $scope.todos.splice(i, 1);
            i--;
        };
    });

    if($scope.todos.length == 0) {
        $scope.isEmpty = true;
    };
}

This adjusts your index to maintain its validity each time your array is modified. You can still use angular.forEach, and you don't end up with two copies of your array.