Formatting data for use in a controller

Question

I am using the function inputSetup to prepare an array for a multi-select directive. I am passing in an array with the existing selections ($scope.item.categories), an array with all the available categories, and a string the carries the context of the values "category".

1. Formatting the array to add the value "ticked" = true; on each object, so that the multi-select understand which options are selected.

2. Concatenating the arrays.

3. Sorting the arrays.

4. Using a for loop to iterate through the array and removing any duplicates that have the same "category" key... then removing the one that has "ticked" = false.

This code feels like it can be improved.

1. Should the .concat and .sort be chained together?
2. Should the for loop be replaced with an array.filter()?

$scope.categories = inputCategories($scope.item.categories, $scope.settings.categories, "category");
function inputSet (input, settings, category) {
 var updatedSettings = [];
 angular.forEach(input, function(obj) {
 var setting = { "ticked": true };
 setting[category] = obj;
 updatedSettings.push(setting);
 });
 var list = updatedSettings.concat(settings);
 list.sort(function(a, b) {
 return (a[category] > b[category]) - (a[category] < b[category]);
 });
 for ( var i = 1; i < list.length; i++ ){
 if(list[i-1][category] == list[i][category]) {
 list.splice(i,1);
 }
 }
 return list;
};

Answer 1

Here's my take on your code. Longer, but in my opinion, easier to follow since it is written as a chain of operations on arrays.

In addition, I use array methods, which easily chain together. Their callbacks are moved out for clarity. They're also written in a way that a function doesn't use outside variables. Makes your code more predictable and easily modifiable without dragging along other stuff.

// The first phase of your code is just mapping obj into another form. Use map().
function inputMapper(obj) {
 var setting = {};
 // Using the bracket notation to keep it uniform.
 setting['ticked'] = true;
 setting[category] = obj;
 return setting;
}
// Removing items first will make further operations iterate less.
// Additionally, we use reduce to carry around the data, and the already-found categories
function duplicateCategoryRemover(carry, item) {
 if (!~carry.categoriesFound.indexOf(item.category)) {
 carry.items.push(item);
 carry.categoriesFound.push(item.category);
 }
 return carry;
}
// Nothing special, just your sorter moved out.
function categorySorter(a, b) {
 return (a[category] > b[category]) - (a[category] < b[category]);
}
// Now everything's in place, let's do the magic.
function inputSet(input, settings, category) {
 return input.map(inputMapper)
 .concat(settings)
 .reduce(duplicateCategoryRemover, { items: [], categoriesFound: [] })
 .items
 .sort(categorySorter);
};

• John Spiteri: Your approach is high level art, compared to my children's drawings... and I have learned over ten theoretical improvements to programming. The one that stands out the most is that dividing the code into such a neat separation of function and action - really means that although longer (slightly), it is so much move valuable, maintainable and reusable. Thank you @Joseph the Dreamer