getify / Functional-Light-JS

Pragmatic, balanced FP in JavaScript. @FLJSBook on twitter.
http://FLJSBook.com
Other
16.6k stars 1.96k forks source link

"compose" function implementation bugs in chapter 4. #146

Closed darkamenosa closed 6 years ago

darkamenosa commented 6 years ago

This afternoon, I tried implement myself a compose function follow this:

function compose(...fns) {
    return function composed(result){
        return fns.reverse().reduce( function reducer(result,fn){
            return fn( result );
        }, result );
    };
}

// or the ES6 => form
var compose = (...fns) =>
    result =>
        fns.reverse().reduce(
            (result,fn) =>
                fn( result )
            , result
        );

I found that this compose function always give me wrong result. The reason is that reverse function mutates the fns array and our fns array is kept in closure.

Here is my code snippet:

const computeTaxForIncome = compose(computeTax, computeTaxedIncome);

console.log(computeTaxForIncome(14000000));
console.log(computeTaxForIncome(18000000));
console.log(computeTaxForIncome(20000000));
console.log(computeTaxForIncome(25000000));
console.log(computeTaxForIncome(50000000));

Stack trace when run the computeTaxForIncome function:

[ [Function: computeTax], [Function: computeTaxedIncome] ]
14000000
5000000
250000
[ [Function: computeTaxedIncome], [Function: computeTax] ]
18000000
1950000
-7050000
[ [Function: computeTax], [Function: computeTaxedIncome] ]
20000000
11000000
900000
[ [Function: computeTaxedIncome], [Function: computeTax] ]
25000000
3350000
-5650000
[ [Function: computeTax], [Function: computeTaxedIncome] ]
50000000
41000000
7000000

My suggestion is adding slice function to the implementation to avoid mutating the fns array.

var compose = (...fns) =>
    result =>
        fns.slice().reverse().reduce(
            (result,fn) =>
                fn( result )
            , result
        );

I will make a pull request for this issue when I have free time.