4

I am having trouble with getting an incorrect value from an array flatten function when using functional programming principals to generate the flatten function from a generic reduce function. I believe that this is because there is an issue with the recursion within the call, but am unsure how to move past it as the function signatures for both the working and non working function should be the same.

Thanks for any help.

var data = [['one','two','three'], ['four', 'five', ['six']], 'seven', ['eight', 'nine']];

// here is an example of flatten that works perfectly. it takes an array and reduces 
// the internal arrays to a single flat array
function flatten( arr ){
  return arr.reduce(function( ret, curr ){
    if( Array.isArray( curr ) ){
      ret = ret.concat( flatten( curr ) );
    } else {
      ret.push( curr );
    }
    return ret;
  }, []);
}

// here is what I am trying to achieve. This one combines my reduction functon with the 
// functional `reduceWith` function. The code signature is exactly the same, however the
// end result is different.
// `functionalFlatten` does resolve to the correct function inside
var functionalFlatten = reduceWith(function( ret, curr ){
  if( Array.isArray( curr ) ){
    ret = ret.concat( functionalFlatten( curr ) );
  } else {
    ret.push( curr );
  }
  return ret;
}, []);

// this function will return a functional reduction function 
function reduceWith( fn, initial ) {
  return function _reduceWith( arr ) {
    return Array.prototype.reduce.call(arr, fn, initial || []);
  }
}

console.log('data', data);
console.log('functionalFlatten', functionalFlatten );
console.log('normal', flatten( data ));
console.log('fuctional', functionalFlatten( data ));
<script src="http://codepen.io/synthet1c/pen/WrQapG.js"></script>
synthet1c
  • 6,152
  • 2
  • 24
  • 39

3 Answers3

3

Here's how I fixed your function

var functionalFlatten = reduceWith(function f( ret, curr ){
  if( Array.isArray( curr ) ){
    ret = ret.concat( reduceWith(f, [])( curr ) );
  } else {
    ret.push( curr );
  }
  return ret;
}, []);

The problem with your initial code was the use of the same initial value for both the parent call and the recursive one which made ret to concatenate with itself.

  • I don't know why you've been downvoted, but I upvoted to balance back--this is a good answer. To OP, its even more evident if you insert a non-empty array in as the `initial` argument to `functionalFlatten`, you can see that `functionalFlatten` is being invoked each time it encounters a new array, hence the duplication pattern in your result. – bazeblackwood Dec 31 '15 at 21:03
  • 1
    @stefan thanks for your answer. The thing I am unsure of is that the `functionalFlatten` should be created an compile time and should be and is available to the inner function. I didn't reference the function as you suggested for that reason. Also looking at this it seems less efficient as you are declaring a new flatten with each level of recursion not reusing the same function. – synthet1c Dec 31 '15 at 21:07
  • @Stefan: That's not the root of the problem. `reduceWith` only gets called once (when `functionalFlatten` is defined). The inner function will get called every time, but `ret.push(curr)` has the potential to mutate `initial`. – demux Dec 31 '15 at 21:42
2

I made a few changes to your code. This works:

var data = [
  ['one','two','three'],
  ['four', 'five', ['six']],
  'seven',
  ['eight', 'nine']
]

function flatten(arr) {
  return arr.reduce(function(ret, curr) {
    return ret.concat(Array.isArray(curr) ? flatten(curr) : [curr])
  }, [])
}

var functionalFlatten = reduceWith(function(ret, curr) {
  return Array.prototype.concat.call(ret, Array.isArray(curr) ? functionalFlatten(curr) : [curr])
}, [])

// I assume you want to use .call to keep it functional or what ever
// But I would just do it like this:
var _functionalFlatten = reduceWith(function(ret, curr) {
  return ret.concat(ret, Array.isArray(curr) ? functionalFlatten(curr) : [curr])
}, [])

function reduceWith(fn, initial) {
  return (function (arr) {
    return Array.prototype.reduce.call(arr, fn, initial) 
  })
}

// Again, keep it simple...
function _reduceWith(fn, initial) {
  return (function (arr) {
    return arr.reduce(fn, initial)
  })
}

// You had this...
function reduceWith( fn, initial ) {
  // You don't need to name this function:
  return function _reduceWith( arr ) {
    // Let's keep this in line original function, so remove the default:
    return Array.prototype.reduce.call(arr, fn, initial || []);
  }
}

console.log('data', data)
console.log('functionalFlatten', functionalFlatten)
console.log('normal', flatten(data))
console.log('fuctional', functionalFlatten(data))

Now to the actual problem...

var functionalFlatten = reduceWith(function( ret, curr ){
  if( Array.isArray( curr ) ){
    ret = ret.concat( functionalFlatten( curr ) );
  } else {
    // This is your culprit:
    ret.push( curr ); // push will mutate ret
  }
  return ret;
}, []);
  • reduceWith only gets called once (when functionalFlatten is defined).
  • The inner function will get called every time
  • ... but ret.push(curr) has the potential to mutate initial

Here is proof...

function reduceWithMutationSafe(fn, initial) {
  return (function (arr) {
    // Clone initial, so that the original can't be mutated:
    var clonedInitial = eval(JSON.stringify(initial))
    return arr.reduce(fn, clonedInitial)
  })
}

var functionalFlatten = reduceWithMutationSafe(function(ret, curr) {
  if(Array.isArray(curr)) {
    ret = ret.concat(functionalFlatten(curr))
  } else {
    ret.push(curr)
  }
  return ret
}, [])

This will work, even though functionalFlatten is exactly the same as before
ret.push(curr) will mutate the cloned initial, but the original one won't be touched.

But this last piece of code is just proof. There should be no need to use reduceWithMutationSafe.

demux
  • 4,544
  • 2
  • 32
  • 56
  • 1
    thanks for taking the time to go through the code and pointing out the areas it can be improved. – synthet1c Dec 31 '15 at 21:17
  • 1
    Once again thanks @Arnar. The explanation and the proof you have provided makes perfect sense. – synthet1c Dec 31 '15 at 22:19
  • 1
    I would also recommend another way of checking if a variable is an `array`. See: http://stackoverflow.com/questions/767486/how-do-you-check-if-a-variable-is-an-array-in-javascript – demux Dec 31 '15 at 22:35
1

Here you can write reduceWith as a curried function which gives you better re-use of the overall function.

// ES6
const reduce = f => y => xs => xs.reduce (f, y);

Now we can write flatten as a partially applied reduce function

const flatten = reduce ((y,x) => y.concat (isArray (x) ? flatten (x) : x)) ([]);

That little isArray helper is just

const isArray = Array.isArray;

And it just works. No mutation via .push, no Function.prototype.call. Just folding and concat'ing.

console.log (flatten ([1,2,[3,4,[],6,[7,8,9]]]));
//=> [1,2,3,4,5,6,7,8,9]

Here's the ES5

// ES5
"use strict";

var reduce = function reduce(f) {
  return function (y) {
    return function (xs) {
      return xs.reduce(f, y);
    };
  };
};

var isArray = Array.isArray;

var flatten = reduce(function (y, x) {
  return y.concat(isArray(x) ? flatten(x) : x);
})([]);

console.log(flatten([1, 2, [3, 4, [], 6, [7, 8, 9]]]));
//=> [1,2,3,4,5,6,7,8,9]
Mulan
  • 129,518
  • 31
  • 228
  • 259
  • He Naomik, thanks for your reply. the only thing I'm not sure about is not using `Array.prototype.reduce.call` I do that so any array like object can be passed in and reduced / mapped / filtered etc.. It may be overthinking it though. – synthet1c Jan 02 '16 at 17:49
  • @synthet1c: Can you name an example of when `foo.reduce` would not work and `Array.prototype.reduce.call` would? – demux Jan 02 '16 at 22:26
  • when you use an array like object eg, arguments ```var arrayLike = { 0: 'zero' , 1: 'one' , 2: 'two' , 3: 'three' , length: 4 }; someReductionFunction( arrayLike );``` if your using an array like object as long as it has numerical indexed keys and a length property you can borrow methods from `Array.prototype` – synthet1c Jan 03 '16 at 00:38
  • 1
    @synthet1c instead of writing all of your functions to support multiple datatypes, I would make a function to convert one to another: `toArray(arrayLike) //=> [...]`. `arguments` is the most common array-like I encounter in JavaScript and that's going away in ES6 thanks to [rest parameters](https://babeljs.io/docs/learn-es2015/#default-rest-spread). `function foo (...xs) { doSomething(xs); }` instead of `function foo() { doSomething(arguments); }` I never liked the implicit behaviour of `arguments` and I'm happy this change is around the corner. – Mulan Jan 03 '16 at 17:54
  • 1
    @naomik thanks for following up. I think you have presented a win win. Less code and it's cleaner. I like it. Yeah I'm really looking forward to `...rest` instead of `slice(args[, startIndex ])`, and the rest of the sugar es6+ will provide. – synthet1c Jan 04 '16 at 17:00