Displaying an item from an array on page load This piece of Angular code is used to add/remove an item to a favourite array and display it on page load. Firstly I check if I have an array and then create one if not. It also checks the localStorage on page load.

How can this be improved?

$scope.favourites = $localStorage.favourites;

$scope.addExerciseToFavourites = function(exercise) {
  if($scope.favourites === undefined) {
    $scope.favourites = [];
    favouriteToggle(exercise)
  } else {
    favouriteToggle(exercise);
  };
};

function favouriteToggle(exercise) {
  if($scope.favourites.indexOf(exercise.exerciseName) == -1) {
      $scope.favourites.push(exercise.exerciseName);
  } else {
      $scope.favourites.splice($scope.favourites.indexOf(exercise.exerciseName),1);
  };
   $localStorage.favourites = $scope.favourites;
};
It's a good start, but there are some improvements that can be made.

For instance, this chunk:

if($scope.favourites === undefined) {
  $scope.favourites = [];
  favouriteToggle(exercise)
} else {
  favouriteToggle(exercise);
};


Could be written as:

if($scope.favourites === undefined) {
  $scope.favourites = [];
}

favouriteToggle(exercise);


Since you always want to call the favouriteToggle function.

But you can go further, and treat undefined as simply another false'y value like false, null, empty string, and numeric zero:

if(!$scope.favourites) {
  $scope.favourites = [];
}
// ...


Which is synonymous with a common set-default-value-if-false'y idiom:

$scope.favourites = $scope.favourites || [];
// ...


You might also see it written as:

$scope.favourites || ($scope.favourites = []);
// ...


The three code blocks above are all equivalent: If $scope.favourites is false'y, set it to [].

Since it's a simple one-liner, you can just move it out of the function entirely, and then just skip the entire function:

$scope.favourites = $localStorage.favourites || [];

function $scope.favouriteToggle(exercise) {
  if($scope.favourites.indexOf(exercise.exerciseName) == -1) {
      $scope.favourites.push(exercise.exerciseName);
  } else {
      $scope.favourites.splice($scope.favourites.indexOf(exercise.exerciseName),1);
  };
   $localStorage.favourites = $scope.favourites;
};


As for favouriteToggle, I'd name it toggleFavourite instead, just because it sounds more natural. But let's make it little more descriptive, and call it toggleExerciseFavourite.

Secondly, instead of finding the index twice with indexOf, it'd be easier to find it once, and store the result:

$scope.toggleExerciseFavourite = function(exercise) {
  var index = $scope.favourites.indexOf(exercise.exerciseName);

  if(index === -1) {
    $scope.favourites.push(exercise);
  } else {
    $scope.favourites.splice(index, 1);
  }

  $localStorage.favourites = $scope.favourites;
};


Lastly, I don't know Angular, I'm afraid, so I don't know how manipulating the $localStorage object works. But if $scope.favourites is just a reference to it, we can skip the last line, since we're manipulating the same object already:

$scope.toggleExerciseFavourite = function(exercise) {
  var index = $scope.favourites.indexOf(exercise.exerciseName);

  if(index === -1) {
    $scope.favourites.push(exercise);
  } else {
    $scope.favourites.splice(index, 1);
  }
};