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

Question

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>

Please elaborate on "everything breaks". What is the expected behavior and what is the actual behavior? — Patrick
– Patrick, Commented Apr 28, 2016 at 9:10
Edited my post. If you check the codepen and click, the stars, I think it would be easier to understand! — Stian Bakken
– Stian Bakken, Commented Apr 28, 2016 at 9:26

Patrick · Accepted Answer · 2016-04-28 11:57:29Z

1

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)
  }
}

edited Apr 28, 2016 at 11:57

answered Apr 28, 2016 at 9:38

Patrick

18k6 gold badges73 silver badges85 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Stian Bakken Over a year ago

Thank you so much! Seems I need to go back to basics before I continue with my angular-skills..

Patrick Over a year ago

@StianBakken; np. Remember that you can optimize it further since in the current case you are looping through the list of favorites twice.

Collectives™ on Stack Overflow

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

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related