Stian Bakken Stian Bakken - 7 months ago 28
Javascript Question

Adding and removing items from arrays, based on a condition in AngularJS

I'm trying to make a dynamic "favorites" toggle-button.
When clicking the button, it should add the selected player to the users favorite-list. If the player is already favorited, it should remove it.

I've also tried to iterate through the favorites, to check if a player is already favorited. If true, it colors the favorite star gold.

A few problems here. My for loop for checking seems to be working properly as long as the array only contains one item. But as soon as I try adding more, the gold icon is only gold colored on the last player added to favorites. So the check only finds one favorite at a time, and I can add a player to favorites many times, as long as I vary the players I add.

If someone could point me in the right direction and help me understand why my loop isn't working correctly, that would be awesome!

http://codepen.io/utrolig/pen/LNgRwv

Javascript

angular.module('test', [])

.controller('TestController', function($scope){
$scope.players = [
{
uniqueid: "gem",
name: "Ole Christian",
cake: false,
},{
uniqueid: "utrolig",
name: "Stian",
cake: false,
},{
uniqueid: "drozo",
name: "Adrian",
cake: false,
}
];

$scope.user = {
name: "Stian",
username: "stiba",
favorites: [{uniqueid: "drozo"}],
}

$scope.checkFavorite = function(id){
fav = $scope.user.favorites;
var exists;
for (var i=0; i < fav.length; i++){
if(fav[i].uniqueid == id){
exists = true;
} else {
exists = false;
}
}
return exists;
}

$scope.toggleFavorite = function(id){
fav = $scope.user.favorites;
if(fav.length === 0){
var newfav = {uniqueid: id};
fav.push(newfav);
} else {
if($scope.checkFavorite(id) === true){
for(var i = 0; i < fav.length; i++){
if (fav[i].uniqueid === id) fav.splice(i, 1);
}
} else if ($scope.checkFavorite(id) === false) {
var newfav = {uniqueid: id};
fav.push(newfav)
} else {
console.log('Error!');
}
}
}

$scope.isFavorited = function(){
return true;
};

})


HTML

<body ng-app="test">
<div class="container" ng-controller="TestController">
<h3>Players</h3>
<div ng-repeat="player in players" class="player-cont">
<div class="player">
<div class="favorite" ng-click="toggleFavorite(player.uniqueid)" ng-class="{'active': checkFavorite(player.uniqueid)}">
<i class="material-icons">star</i>
</div>
<i class="material-icons player-icon">person</i>
</div>
<div>
<p ng-bind="player.uniqueid"></p>
<p ng-bind="player.name"></p>
</div>
</div>
<h3>Favorites</h3>
<div ng-repeat="favorite in user.favorites track by $index">
<h5>{{favorite.uniqueid}}</h5>
</div>
<p class="user">
{{user.favorites}}
</p>
</div>

</body>

Answer

There's a couple of errors in your code.

The first is checkFavorite, if you examine the code you'll see that only the last item is actually compared to id, since the exists flag is updated for each item. You need to "short circuit" the loop and return true as soon as you find a value.

btw, is* is a common name convention for checking boolean values.

$scope.isFavorite = function(id){
  var fav = $scope.user.favorites;
  for (var i=0; i < fav.length; i++){
    if(fav[i].uniqueid == id){
      return true;
    }
  }
  return false;
}

Your toggle is also very verbose, if you "reduce" the code you end up with something like this

$scope.toggleFavorite = function(id){
  var fav = $scope.user.favorites;
  // no previous items, remove, OK
  if(fav.length === 0) {
    var newfav = {uniqueid: id};
    fav.push(newfav);
    return;
  }

  // if already a favorite, uncheck/remove
  if($scope.isFavorite(id)) {
    for(var i = 0; i < fav.length; i++){
       if (fav[i].uniqueid === id) fav.splice(i, 1);
    }
  }
  // otherwise add the item
  // remember, isFavorite returns true of false, there is no third state
  else { // if ($scope.isFavorite(id) === false) {
    var newfav = {uniqueid: id};
    fav.push(newfav)
  }
}

This can be edited further, since the isFavorite function will return false if the list is empty, i.e. no need for the first if

$scope.toggleFavorite = function(id){
  var fav = $scope.user.favorites;
  // if already a favorite, uncheck/remove
  if($scope.isFavorite(id)) {
    for(var i = 0; i < fav.length; i++){
       if (fav[i].uniqueid === id) {
         fav.splice(i, 1);
         // unless the item exists more than once, break the loop
         break;
       }
    }
  }
  // otherwise add the item
  else {
    var newfav = {uniqueid: id};
    fav.push(newfav)
  }
}