CODEWITHSUNDEEP
javascriptobjectfor-looparray-map
Dave
Dave

Reputation: 1277

How to map within a for loop

We need to map an object array within a for loop, which actually works, but the editor is giving us a warning saying not to put a function within a loop:

for(var i=0; i<$scope.data.list.length; i++){
    $scope.data.list[i].isRowSelected=false;

    var pos1 = $scope.selectedItems.map(function(e) { return e.sys_id; }).indexOf($scope.data.list[i].sys_id);
    if(pos1!==-1){
        var add = $scope.selectedItems.indexOf($scope.data.list[i].sys_id);
        $scope.selectedItems.splice(add,1);
    }
}

To mitigate this, we're thinking about creating a separate function for the mapping and then calling it within the loop, like this:

function mappingID(e){
  return e.sys_id;
}

However, when we call upon it within the loop, we're lost as to what to pass in...any suggestions? Thanks!

Upvotes: 0

Views: 1690

Answers (4)

georgeawg
georgeawg

Reputation: 48968

To avoid doing the same mapping on each iteration of the loop, move the mapping outside the loop:

var idArr = $scope.selectedItems.map(function(e) { return e.sys_id; })

$scope.data.list.forEach(item => {
    item.isRowSelected=false;

    var pos1 = idArr.indexOf(item.sys_id);
    if(pos1!==-1){
        var add = $scope.selectedItems.indexOf(item.sys_id);
        $scope.selectedItems.splice(add,1);
    }
})

Upvotes: 0

Keith Nicholas
Keith Nicholas

Reputation: 44288

two things, create a function outside the loop and avoid repeating indexing and object nesting. It will make your code much cleaner and easier to reason about. I'm pretty sure this whole function could be done a lot better but I'm not sure of the bigger scope

var items = $scope.selectedItems;
var sys_id = function(e) { return e.sys_id; }
for(var i=0; i<$scope.data.list.length; i++){
    var data = $scope.data.list[i];  // might be a better name for this...
    data.isRowSelected=false;

    var pos1 = items.map(sys_id).indexOf(data.sys_id);
    if(pos1!==-1){
        var add = items.indexOf(data.sys_id);
        items.splice(add,1);
    }
}

Upvotes: 1

Matt Aft
Matt Aft

Reputation: 8936

You don't need to bring lodash to handle this, you can use find: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find

for(var i=0; i<$scope.data.list.length; i++){
    $scope.data.list[i].isRowSelected=false;

    var item = $scope.selectedItems.find(e => (e.sys_id === $scope.data.list[i].sys_id));

    if (item) {
        $scope.selectedItems.splice(item,1);
    }
}

Also I suggest changing selectedItems to an plain-object/Map/Set so you can lookup in constant time.

Upvotes: 1

see sharper
see sharper

Reputation: 12025

The comments suggest lodash, which is a good suggestion. For the purposes of your original question, however, you can declare the function mappingID as you have it, and simply put

var pos1 = $scope.selectedItems.map(mappingID).indexOf($scope.data.list[i].sys_id);

and that will do the job.

Upvotes: 1

Related Questions