2
\$\begingroup\$
const generateFibonnaciSequence = (n) => {
    return [
        (arr = new Array(n).fill(1).reduce((arr, _, i) => {
            arr.push(i <= 1 ? 1 : arr[i - 2] + arr[i - 1]);
            return arr;
        }, [])),
        arr[n - 1],
    ];
};

const [fibonacciSequence, fibonacciNthNumber] = generateFibonnaciSequence(n);

My idea is to return an array holding fibonacci sequence up to n in index 0 and fibonnaci's n-th value in index 1. Can someone help me with a prettier way of constructing this function. I'd like to avoid holding the array in temp arr variable if possible, but still use a single expression for return statement.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ push method mutates the data. A cool improvement would be to use the ... ES6 spread syntax in order to create this array in the return statement \$\endgroup\$
    – AdamKniec
    Feb 26 '21 at 22:45
1
\$\begingroup\$

Quick review

Try to avoid very long names "generateFibonnaciSequence" can be "fibonnaciSequence".

The new array arr is undeclared and thus will be in the global scope. Chrome has recently improved how it handles optional parameters so there is no penalty to declare the array as an argument scoped to the function.

It seems to me that the reducer can return the last Fibonnaci number rather than a array which will make the code a little cleaner.

You can also reduce the iteration by one because you are starting the sequence at 1 rather than 0.

Filling the array is a lot of extra work as you only need the first value set.

Putting all that together you can so it all with just the one array (ignoring the returned array)

const fibSeq = (n, arr = new Array(n - 1)) => [
    arr, arr.reduce((fib, _, i) => arr[i + 1] = i ? fib + arr[i - 1]: 1, arr[0] = 1)
];

The reduction in overheads makes it run about 4 times quicker.

However there is a dangerous behavioral change. If n is 0 it will throw when trying to create an array with a negative value, and n = 1 will return [[1,1], 1] rather than [[1], 1].

If you need it to return for these values you can check for 0 and 1 returning the static values when needed.

const fidSeq = (n, a = --n > 0 && new Array(n)) => 
    a? [a, a.reduce((f, _, i) => a[i+1] = i? f + a[i-1]: 1, a[0] = 1)]: n? [[],]: [[1], 1];
\$\endgroup\$
1
  • \$\begingroup\$ fidSeq made me giggle. Also, that last piece of code belong on CodeGolf, not CodeReview. \$\endgroup\$
    – konijn
    Feb 28 '21 at 9:49
2
\$\begingroup\$

A short review;

  • I prefer fat arrow syntax => for inline functions
  • Its a good exercise to code this with reduce but it is definitely not the best option for production code. (That would be a non-recursive old skool loop)
  • The modern approach to Fibonacci numbers has them start with [0,1,1] not [1,1]
  • If you have control over what the function returns, then I would just return the sequence. It is trivial for the caller to get the last value in the array

Please find below a counter proposal, I like that it support both the old and new style sequence.

function buildFibonnaciSequence(n){
  const start = [0,1];
  //Recursion exit lane
  if(n <= 1){
    return [start.slice(0,n+1), start[n]];
  }
  //Recurse
  const priorSequence = buildFibonnaciSequence(n-1);
  //Build return value
  const nValue = priorSequence[0][n-2] + priorSequence[0][n-1];
  return [[...priorSequence[0], nValue], nValue];
}

console.log(buildFibonnaciSequence(0));
console.log(buildFibonnaciSequence(1));
console.log(buildFibonnaciSequence(5));
console.log(buildFibonnaciSequence(45));

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.