patternjavaMinor
Calculate fibonacci in O(log n)
Viewed 0 times
calculatefibonaccilog
Problem
This program calculates the \$n\$th fibonacci number, in \$O(\log n)\$ time. I'm looking for code review, optimizations, and best practices.
public final class Fibo {
private Fibo() { }
public static int getNthfibo(int n) {
if (n 0) {
if (n%2 == 1) {
multMatrix(result, fiboM);
}
n = n / 2;
multMatrix(fiboM, fiboM);
}
return result[1][0];
}
private static void multMatrix(int[][] m, int [][] n) {
int a = m[0][0] * n[0][0] + m[0][1] * n[1][0];
int b = m[0][0] * n[0][1] + m[0][1] * n[1][1];
int c = m[1][0] * n[0][0] + m[1][1] * n[0][1];
int d = m[1][0] * n[0][1] + m[1][1] * n[1][1];
m[0][0] = a;
m[0][1] = b;
m[1][0] = c;
m[1][1] = d;
}
public static void main(String[] args) {
for (int i = 0; i < 10; i++) {
System.out.println(getNthfibo(i));
}
}
}Solution
Your code seems to be working properly so there is not much to say on this.
The only changes I would perform would be to make things easier to understand through different little steps.
-
Multiplication could return a result
At the moment, your multiplication procedure stores the result in the first argument. First issue is that it cannot be generalised to product of non-square matrices. The second issue is that it can make things a bit hard to understand when looking at the code using your function (
and in your main function:
-
The signature of the
Actually, it only works for 2*2 matrices. This should be documented and checked at runtime (because it doesn't seem to be possible to do so at compilation time).
-
Making Mathematics a bit more obvious
Values can be reformatted in such a way that the mathematics behind are easier to understand. Also, a little bit of comment can help.
The only changes I would perform would be to make things easier to understand through different little steps.
-
Multiplication could return a result
At the moment, your multiplication procedure stores the result in the first argument. First issue is that it cannot be generalised to product of non-square matrices. The second issue is that it can make things a bit hard to understand when looking at the code using your function (
getNthfibo in your case). It is a fairly easy change to perform :private static int[][] multMatrix(int[][] m, int [][] n) {
int a = m[0][0] * n[0][0] + m[0][1] * n[1][0];
int b = m[0][0] * n[0][1] + m[0][1] * n[1][1];
int c = m[1][0] * n[0][0] + m[1][1] * n[0][1];
int d = m[1][0] * n[0][1] + m[1][1] * n[1][1];
int[][] ret = {
{a, b},
{c, d}};
return ret;
}and in your main function:
result = multMatrix(result, fiboM);` and `fiboM = multMatrix(fiboM, fiboM);-
The signature of the
multMatrix method lets us think that is works for any matrices.Actually, it only works for 2*2 matrices. This should be documented and checked at runtime (because it doesn't seem to be possible to do so at compilation time).
-
Making Mathematics a bit more obvious
Values can be reformatted in such a way that the mathematics behind are easier to understand. Also, a little bit of comment can help.
int[][] result = {
{1, 0},
{0, 1}};
/* n
* [ 1 1 ] [ F(n+1) F(n) ]
* [ 1 0 ] = [ F(n) F(n-1) ]
*/
int[][] fiboM = {
{1, 1},
{1, 0}};Code Snippets
private static int[][] multMatrix(int[][] m, int [][] n) {
int a = m[0][0] * n[0][0] + m[0][1] * n[1][0];
int b = m[0][0] * n[0][1] + m[0][1] * n[1][1];
int c = m[1][0] * n[0][0] + m[1][1] * n[0][1];
int d = m[1][0] * n[0][1] + m[1][1] * n[1][1];
int[][] ret = {
{a, b},
{c, d}};
return ret;
}result = multMatrix(result, fiboM);` and `fiboM = multMatrix(fiboM, fiboM);int[][] result = {
{1, 0},
{0, 1}};
/* n
* [ 1 1 ] [ F(n+1) F(n) ]
* [ 1 0 ] = [ F(n) F(n-1) ]
*/
int[][] fiboM = {
{1, 1},
{1, 0}};Context
StackExchange Code Review Q#51864, answer score: 7
Revisions (0)
No revisions yet.