Matching and pushing array

Question

This code works, but I feel like it's really hacky and could be done a lot better. Can anyone give some advice on how this could be neater?

$scope.arrays = function() {
        $scope.committees = [];
        $scope.employees = [];

        $http({method: 'GET', url: 'data.php'}).success(function(data) {
            angular.forEach(data, function(value, key) {
                $scope.committees.push(value);
            })

            $http({method: 'GET', url: 'data2.php'}).success(function(data){
                angular.forEach(data, function(value, key) {
                    $scope.employees.push(value);
                })
                $scope.arr = new Array();
                for (var i = 0, committee; committee = $scope.committees[i]; i++) {
                    for (var j = 0, employee; employee = $scope.employees[j]; j++) {
                            if (committee.code === employee.code){
                                $scope.arr.push({
                                    "committee" : committee.name,
                                    "color": committee.color,
                                    "members": committee.members,
                                    "femaleAnnual" : employee.femaleAnnual,
                                    "maleAnnual" : employee.maleAnnual,
                                    "totalAnnual" : employee.totalAnnual,
                                })
                            }
                    }
                }
            })
        })
    }

    $scope.arrays();

Edit: Sorry I should have been clearer! What I'm trying to do here is merge 2 arrays if they have the same value for 'code' - and then push that array to the scope.

Hi! Welcome to Code Review. It would help reviewers more if you include more context in your question, especially what your code does. Thanks! — TheCoffeeCup
– TheCoffeeCup, Commented Jun 2, 2015 at 22:48

Joseph · Accepted Answer · 2015-06-03 00:12:42Z

You could take advantage of the fact that push accepts more than one argument so you only call push once. Build the arguments array and use apply.

Array.prototype.push.apply($scope.committees, Object.keys(data).map(function(key){
  return data[key];
}));

Or if you're just putting values instead of actually appending to the existing array:

$scope.committees = Object.keys(data).map(function(key){
  return data[key];
});

Now for the AJAX part, you don't need to have a sequential request, that's that last thing you'd want to do in AJAX (besides making them synchronous). You'll need something like Promise.all or jQuery's $.when, fire them at the same time and resolve when all is done.

var comitteePromise = $http({method: 'GET', url: 'data.php'});
var employeePromise = $http({method: 'GET', url: 'data2.php'});

Promise.all([comitteePromise, employeePromise]).then(function(values){
  var committees = values[0];
  var employees = values[1];
  ...
});

The further I look into your code, what you're just doing is merging the data from both arrays. You can build a lookup for one array, and iterate the other. That way, two loops and not to the power of two loops.

var comitteePromise = $http({method: 'GET', url: 'data.php'});
var employeePromise = $http({method: 'GET', url: 'data2.php'});

Promise.all([comitteePromise, employeePromise]).then(function(values){
  var committees = values[0];
  var employees = values[1];

  // Use the committee code (assuming it's unique) as a key for lookup
  var committeesByCode = committees.reduce(function(carry, committee){
    carry[committee.code] = committee;
    return carry;
  }, {});

  // For each employee, lookup the committee and add in the data
  $scope.arr = employees.map(function(employee){
    var committee = committeesByCode[employee.code];
    return {
      committee: committee.name,
      color: committee.color,
      members: committee.members,
      femaleAnnual: employee.femaleAnnual,
      maleAnnual: employee.maleAnnual,
      totalAnnual: employee.totalAnnual
    };
  });

});

This is great, thanks for the explanation - works much faster than my original code. — dave
– dave, Commented Jun 3, 2015 at 20:30

Stack Exchange Network

Matching and pushing array

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Matching and pushing array

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions