# Better way to write this code in functional manner using map and reduce?

I have an array of items on which I have to perform 2 tasks:

1. Applying a function on the array item
2. Checking if the value is true or false.

The functional approach to solving this would be

var result = arr.map(function(item){return some_action(item);}).filter(function(item){return other_action(item) == true;});


But here the array arr is traversed twice in comparison to

var result = [];
arr.forEach(function(item){
var x = other_action(some_action(item));
if (x)
result.push(x);
});


Isn't the functional approach bad in this case or am I not using map and filter the right way?

-

Your “functional” approach is overly complicated. Notice that this is completely equivalent:

var result = arr.map(some_action).filter(other_action);


I.e. If you're only delegating to another function, you can specify that function directly. Also, an == true test is superfluous.

Your “procedural” variant is not equivalent, that would have to be:

var result = [];
arr.forEach(function(item){
var changedItem = some_action(item);
if (other_action(changedItem))
result.push(changedItem);
});


Note that both variants have the same algorithmic complexity, and that the cost of iteration is likely negligible compared with the cost of some_action and other_action.

Functional programming does not mean unreadable code. Even if you're not just delegating to another function, you could improve formatting, e.g. to

var result = arr.map(function (item) {
return some_action(item);
}).filter(function (item) {
return other_action(item);
});

-
sorry i mistyped the procedural version – lovesh Jan 31 '14 at 12:52

Your loops are not equivalent. In the first one result will be arr mapped with some action, while in the second you could do that just using a filter.

The equivalent way of writing the forEach loop would be to use result.push(some_action(item)). Of course you would cache it as you use some_action(item) earlier in the function.

The equivalent way of writing your forEach loop with just filter would be

arr.filter(function(item){return other_action(some_action(item));});


Also, as this is code review I would like to point out you can probably write

arr.map(function(item){return some_action(item);})


As

arr.map(some_action) //note this is not equivalent if some_action can take multiple params.


Anyway, you are correct in that calling these loop methods will take O(n) time per call and it's a good idea to apply filters first to limit n. If you're looking for an interesting read take a look at the Lazy.js documentation.

Edit, just noticed, because you mention reduce, the way you'd write this with reduce is akin to you forEach way

arr.reduce(function(result, item){
var item = other_action(some_action(item));
if (item) result.push(item);
return result;
}, []);

-

Don't do this:

 var result = arr.map(function(item){
var result=some_action(item);
if(other_action(result)){
return result;
}
});


if other_action() return false, the item position in array will be undefined.

-
When other_action(result) is falsy the mapped value will be undefined which is not desired. \$.map will allow you to do that though – megawac Feb 3 '14 at 4:17
you are right. I am not aware of it. thanks. – caoglish Feb 3 '14 at 7:12