Return to Answer

typos

Source Link

edited May 3, 2015 at 16:56

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

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

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

Lastly, we 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:

But you can go further, and treat undefined as simply another false'y value:

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

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

Lastly, we 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, we can skip the last line, since we're manipulating the same object already:

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

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

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

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:

Source Link

created May 3, 2015 at 13:25

Flambino

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:

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 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, 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;
};

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

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