20

I have this code for calculating fibonacci number in python. It works and gives the expected result. but when I translated the same to Java, it fails. Any idea of what is going wrong here?

In python:

def fib3(n): 
  a,b=0,1
  while n>0:
      a,b=b,a+b
      n-=1
  return a

fib3(12) --> 144

In Java:

 public static int fib2(int n){
        int a = 0;
        int b =1;
        while(n-- >0){
            a=b;
            b=a+b;

        }
    return a;
}

fib2(12) --> 2048

eagertoLearn
  • 9,772
  • 23
  • 80
  • 122
  • 4
    It probably has to do with the fact that the line `a=b` changes the value of `a` before computing `a + b` in the next line. – sdasdadas Feb 05 '14 at 18:52
  • 2
    Nothing to do with python or java. Also in python `a = b; b = a+b` won't work as expected. – Hyperboreus Feb 05 '14 at 19:07

6 Answers6

27

In this section:

a=b;
b=a+b;

you're assigning b to a+b, but a is already b. So really you're doubling b

Easiest solution is a temp variable:

public static int fib2(int n){
    int a = 0;
    int b =1;
    while(n-- >0){
        int old_a;
        old_a = a;
        a=b;
        b=old_a+b;
    }
    return a;
}

In python, a, b = b, a + b stores an intermediate tuple automatically before assigning the new values to the variables, while in Java you need to be explicit about it

Breaking down Python's instructions, a, b = b, a + b is executing this disassembly:

  5          17 LOAD_FAST                1 (b)
             20 LOAD_FAST                0 (a)
             23 LOAD_FAST                1 (b)
             26 BINARY_ADD
             27 ROT_TWO
             28 STORE_FAST               0 (a)
             31 STORE_FAST               1 (b)

In a simpler sense, staying python, here's the process:

temp_tuple = (b, a + b)
a, b = temp_tuple
mhlester
  • 22,781
  • 10
  • 52
  • 75
  • 2
    bah! you beat me by 13 seconds – inspectorG4dget Feb 05 '14 at 18:53
  • can you elaborate the itermediate tuple part a little. I guess that has confused me.. – eagertoLearn Feb 05 '14 at 18:58
  • 1
    certainly. i've updated the answer with some more information – mhlester Feb 05 '14 at 19:02
  • 1
    And I didn't have chance to write answer. Anyway, may I suggest 1 edit? Define old_a inside while. It has no use outside the scope. – Bikas Feb 05 '14 at 19:47
  • @BikasVaibhav, I have followed your suggestion. I don't know Java well, so am unsure of its implications. Is `old_a` now scoped to within `while`? If so, is it re-created in each loop or just on the first pass? – mhlester Feb 05 '14 at 20:49
  • Yes it's recreated every time while loop runs, and destroys once it completes. – Bikas Feb 05 '14 at 20:51
  • Doesn't that seem **less** efficient then? – mhlester Feb 05 '14 at 20:53
  • @mhlester It has at worst a negligible impact on performance. It is generally good practice to declare a variable in the scope it is used, however, as it makes code more easily understood (you don't even need to think about what a_old might be used for outside the loop, since it is out of scope there). – Trying Feb 05 '14 at 23:33
  • @mhlester The code won't be less efficient because compiler is witty enough to optimize it. Most probably the variable old_a won't be created at all and instead of it one of the processor registers will be used. – ciamej Feb 06 '14 at 01:08
  • 1
    The answer could be: *never* copy Python code to Eclipse, make few syntax adaption, and think it could ever work – usr-local-ΕΨΗΕΛΩΝ Feb 06 '14 at 09:34
9

The issue is that you've got to one value from b to a at the same time as assigning to b the sum of a and b. Get that simultaneous swap wrong and you get the wrong answer.

In the spirit of the Python code, I present:

public static int fib(int n) {
    int a = 0, b = 1;
    while (n-->0)
        b = a + (a = b);
    return a;
}

This does the swap effectively at the same time as the add (strictly not, but it's good enough). Note that this is well-defined Java, as the language defines the evaluation order of operators precisely, unlike in C and C++ (where the equivalent of the above code is permitted to make demons fly out of your nose due to being Undefined Behavior).


OK, if it did make you experience problems with nasal demons, I'd suggest not using that compiler in future. But you wouldn't have any assurance of getting a correct fib() function…

Donal Fellows
  • 133,037
  • 18
  • 149
  • 215
  • 4
    For the record, if I see anything that does tricks like that in code that I maintain, I _will_ refactor it into simpler (if more verbose) statements. Excessive trickiness is always a good reason to refactor! – Donal Fellows Feb 05 '14 at 21:49
  • Also, for fib(largevalue) you're often better off using something based on the Gamma function if you need speed. (I always have to look that up when I need it, which is hardly ever.) – Donal Fellows Feb 06 '14 at 07:48
6
a = b;
b = a+b;

This computes b = 2*b because a's value is overwritten by the time you compute the new value for b. Replace it with:

t = b;
b = a+b;
a = t
inspectorG4dget
  • 110,290
  • 27
  • 149
  • 241
5
a=b;
b=a+b;

... is the problem. You need to save the old value of a before adding it to b.

Wolf
  • 4,254
  • 1
  • 21
  • 30
5

The code is not equivalent, and relies on python's ability to assign multiple primitives in one line a,b=b,a+b; you need a temporary variable -

public static int fib2(int n){
  int a = 0;
  int b =1;
  while(n-- >0){
      int t = b; // <-- to hold the original value of b.
      b = a + b;
      a = t;
  }
  return a;
}
Elliott Frisch
  • 198,278
  • 20
  • 158
  • 249
  • a,b=b,a+b in python works as a = b; followed by b = a+b..correct? – eagertoLearn Feb 05 '14 at 18:55
  • 1
    @eagertoLearn Almost. The result of the first expression (b) is assigned to `a` and the second expression (a+b) is assigned to `b`, but the evaluations complete before the assignments. – Elliott Frisch Feb 05 '14 at 18:58
3

In java - When you write "a=b; b=a+b;" you are essentially saying that a should be equal to be and then that (since 'a' is now equal to 'b') that 'b' should just be twice what it originally was. There are two ways you can fix this. 1) You can either continue the function you are originally using and then create a int 'temp' to store 'a' before you change 'a'. 2) You can also do what I would rather do ( this will use a lot more time however for a algorithm like fibonacci and would generally be a terrible idea for real world applications) is use a recursive function that will call itself.

It would generally look something like the following.

    public static int fib2(int n){
           if(n<=0){return 0;}
           if(n<2){return 1;}
           else{ return fib2(n-1)+fib2(n-2);}
    }

That would probably not be the exact code but something very similar. Hope that was helpful!

EFruchter
  • 56
  • 2