9
\$\begingroup\$

I'm attempting to find a fibonacci series where the last number is no greater than a given maximum. Given the start digits 1, 2

Here's my working code.

var fib = [1,2];
var max = 4000000;
var i = 2;
var loop = true

while (loop) {
    if (fib[i-1] + fib[i-2] > max) {
        loop = false;
    } else {
        fib.push(fib[i-1] + fib[i-2]);
        i++;
    }
}

console.log(fib)
\$\endgroup\$
0

3 Answers 3

10
\$\begingroup\$

Your loop can be terminated in a few different ways. You're using a boolean loop variable, but there are cleaner ways.

One is to simply move the if condition to the while condition (Malachi's answer beat me to this):

while (fib[i-1] + fib[i-2] <= max) {
  i = fib.push(fib[i-1] + fib[i-2]);
}

push helpfully returns the array's new length, so you don't need the i++.

Another is to keep the if branching, but use it to break the loop instead:

while (true) {
  var next = fib[i-1] + fib[i-2];
  if (next > max) {
    break; // exit the loop
  }
  i = fib.push(next);
}

Here we avoid calculating the new fibonacci number twice, but still: the previous code block is probably still the cleanest.

You also initialize i to 2, which is a bit redundant: It's the length of the fib array. Rather than manually initializing the array, and manually recording its length, I'd just do var i = fib.length.

Lastly, it might be good to consider "bad" max values. I.e. what if the maximum is set to zero? Or 1? Or -2324359? For zero you could return an empty array; for 1 you'd need to return a shorter array than what you start with, and for the negative max you could throw an error or return null. Not saying you need to any of this, but it's always to good to consider how you handle such cases.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ push returns new length - well that's handy thanks. \$\endgroup\$
    – Luke
    Commented Feb 17, 2017 at 15:06
4
\$\begingroup\$

I think that you can remove the loop variable by moving that equation into the while conditional statement, like this:

var fib = [1,2];
var max = 4000000;
var i = 2;

while (fib[i-1] + fib[i-2] <= max) {
    fib.push(fib[i-1] + fib[i-2]);
    i++;
}

console.log(fib)

I just inverted the condition.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Upvote for the loop simplification, but the other answer use i = push() which I liked and had a slightly clearer explanation. \$\endgroup\$
    – Luke
    Commented Feb 17, 2017 at 15:10
1
\$\begingroup\$

In the interest of providing an alternate from the while loops already given:

var f = (a, b, c, m) => (b + c >= m) ? a : f(a.concat([b + c]), c, b + c, m);
var fib = f([1, 1], 1, 1, 10);

Less pretty version for golfs sake (43 chars)

a=[];for(b=c=1;c<=10&&a.push(b,c);c+=b+=c)a
\$\endgroup\$
2
  • \$\begingroup\$ Woops, thought this was the CodeGolf SE :) \$\endgroup\$
    – John Jones
    Commented Feb 18, 2017 at 1:29
  • \$\begingroup\$ I like it, but I think the while loop is a little more readable for me at the minute. \$\endgroup\$
    – Luke
    Commented Feb 20, 2017 at 9:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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