Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fibonacci sequences #30

Closed
ohepworthbell opened this issue May 29, 2018 · 4 comments
Closed

Fibonacci sequences #30

ohepworthbell opened this issue May 29, 2018 · 4 comments
Labels
enhancement New feature or request

Comments

@ohepworthbell
Copy link
Contributor

ohepworthbell commented May 29, 2018

Not an issue - more of a suggestion. Would be good to return an array for the fibonacci sequence, as well as the sequence at nth position (and maybe rename the nth-position fibonacciNth() or something). For example:

export default function fibonacci(steps) {
  const fibSequence = [];

  let currentValue = 1;
  let previousValue = 0;

  if(steps === 1) {
    return [1];
  }

  while(steps--) {
    currentValue += previousValue;
    previousValue = (currentValue - previousValue);

    fibSequence.push(currentValue);
  }

  return fibSequence;
}

And:

export default function fibonacciNth(steps) {
  let currentValue = 1;
  let previousValue = 0;

  if(steps === 1) {
    return 1;
  }

  while(steps--) {
    currentValue += previousValue;
    previousValue = (currentValue - previousValue);
  }

  return currentValue;
}

Also, where before there was a:

while(iterationsCounter) {
  // Do code
  
  iterationsCounter -= 1;
}

This is mildly verbose, and can be more succinctly written without detriment to legibility as:

while(iterationsCounter--) {
  // Do code
}

Personally I think that's more readable. Happy to do a PR?

Edit: camel cased variable names to match existing code

@trekhleb trekhleb added the enhancement New feature or request label May 30, 2018
@trekhleb
Copy link
Owner

@ohepworthbell thank you for suggestion!
Yes please just create a PR.

@ohepworthbell
Copy link
Contributor Author

Great, pull request created! Had to revert the iterationsCounter to the original loop as JSLint doesn't seem to allow unary operators

@trekhleb
Copy link
Owner

trekhleb commented Jun 2, 2018

Fixed in #36

@trekhleb trekhleb closed this as completed Jun 2, 2018
@hdcorder147
Copy link

Stop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants