This code loops through an array called $rootScope.watchlist
(uses AngularJS). It gets the current iteration of the loop to access that x iteration through $rootScope.watchlist
and get the id
index. If that matches the variable titleID
, then it will splice (remove) that index only and then stop the foreach to stop unnecessarily continuing.
for (var i = $rootScope.watchlist.length - 1; i >= 0; i--) {
if($rootScope.watchlist[i]['id'] == titleID) {
$rootScope.watchlist.splice(i, 1);
break;
}
}
Is there a better way of doing such? Possibly using .some()?
-
\$\begingroup\$ What exactly are you using this for in practice? \$\endgroup\$BenC– BenC2017年09月09日 01:02:07 +00:00Commented Sep 9, 2017 at 1:02
2 Answers 2
A while back, I went through these exercises about functional programming in JS and since then I strive to use those techniques whenever possible. Bear in mind that functional techniques can lead to slower code, since a function call is made for each iteration instead of just running code within a loop block, but hopefully it is easier to work with each array element in that fashion - e.g. would you read/type if($rootScope.watchlist[i]['id'] == titleID) {
or if(element['id'] == titleID) {
?
After reading Igor's answer as well as this article about break
with respect to functional programming, I believe the following should work:
$rootScope
.watchlist
.slice() // copy before reverse(), because reverse() is mutating
.reverse()
.some(function(element, i) {
if(element['id'] == titleID) {
$rootScope.watchlist.splice($rootScope.watchlist.length - 1 - i, 1);
return true;
}
});
Edit:
Igor suggests simplifying that callback passed to some()
. You can use an ES-6 arrow function and a ternary operator to eliminate a few lines, like this:
.some((element, i) => (element['id'] == titleID)? $rootScope.watchlist.splice($rootScope.watchlist.length - 1 - i, 1) : false);
See a demonstration in this plunker. Click the button labeled remove watcher and see how the watcher is removed. This code presumes $rootScope.watchlist
is equivalent to $rootScope.$$watchers
...
-
1\$\begingroup\$ You showed a really useful technique! Nice improvement, Sam! I'd still compress the
.some()
's body to this:.some((element, elementIndex) => element['id'] != titleId ? false : ($rootScope.watchlist.splice($rootScope.watchlist.length - elementIndex - 1, 1), true))
\$\endgroup\$Igor Soloydenko– Igor Soloydenko2017年09月09日 04:08:17 +00:00Commented Sep 9, 2017 at 4:08
Here's what functional style might look like. Unfortunately, it requires index recalculation, and overall more verbose and complicated, even though it tries to go the ".some()" way. :)
I believe it's one of those cases where imperative code keeps things simple.
const indexFromRear =
$rootScope
.watchlist
.slice() // copy before reverse(), because reverse() is mutating
.reverse() // reverse, since the original code searches rear-to-front
.findIndex(element => element['id'] == titleID);
if (indexFromRear >= 0) {
const indexFromFront = $rootScope.watchlist.length - indexFromRear - 1;
$rootScope.watchlist.splice(indexFromFront, 1);
}
Explore related questions
See similar questions with these tags.