Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I would like your opinion on my shell script that prints the Fibonacci sequence.

#!/bin/bash

first=0
second=1
third=1
echo -n "Term: "
read term

fub=$((second+third))
echo $second
echo $third
echo $fub

for ((i=1; i<term; i++))
do
  third=$((fub+third))
  echo $third
  fub=$((third+fub))
  echo $fub
done

I tried to calculate the Fibonacci sequence in the simplest way (without the use of recursive functions).

share|improve this question
    
Just so you know, bash is extremely inefficient for this type of usage. If you want to change system settings, install software, or generally handle anything to do with files, use bash. For computational/algorithmic type problems, not so much. As someone (Xanthir) on the xkcd forums wrote, "The correct way to write anything in Bash is to write it in ANY OTHER LANGUAGE, then call it from a bash script." Which is crude but accurate for computational/algorithmic type problems. – Wildcard Oct 15 '15 at 3:52
    
Yeah, I understand that. BASH is not granular enough for specific mathematical logic. – Bruce Blacklaws Nov 3 '15 at 12:38

What does "Term" mean? I'd prefer to see 5 numbers if the user entered 5.

#!/bin/bash

echo -n 'Term: '
read term

i=0
j=1
for (( c=0; c<term; c++ )) ; do
    (( n=i+j ))
    (( i=j ))
    (( j=n ))
    echo $i
done
share|improve this answer
    
Looks like terminal, terminate or something like that. end then. – Arthur2e5 Oct 25 '15 at 23:07

While I don't write in Bash, I do know that you don't need more than two variables, at the most, to compute the Fibonacci sequence. In fact, you're very close to doing so.

Just sort of manipulating your code here,

#!/bin/bash

# first was never used, it was not required
second=1 # Somewhat confusing name, consider something different
third=1 # Same here.
echo -n "Term: "
read -r term # Using the -r argument was recommended after research

# Notice this functions without the beginning section
for ((i=1; i<term; i++))
do
  echo $second
  second=$((second+third))
  echo $third
  third=$((second+third))
done

If you were to apply this code to your picture above, simply replace all instances of fub with second.


Edit:

The modified code should display 2*term-2 numbers, (Your code as it is now displays more, 2*term+1 numbers)

To change this so it only displays term numbers, change term so it's the proper upper limit after you've read it.

So, the code could look something like this:

for ((i=1; i<term/2+1; i++))

Tested it and yes, the code does work.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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