• Post Reply Bookmark Topic Watch Topic
  • New Topic

Simple Fibonacci (Error is in the method calling & recursion)  RSS feed

 
Greenhorn
Posts: 7
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator



/*
Fib[5] = Fib[3]+Fib[4];
Fib[4] = Fib[2] + Fib[3];
Fib[3] = Fib[1]+Fib[2];
Fib[2] = Fib[0]+Fib[1];
Fib[3] = Fib[1] + Fib[2];
Fib[2] = Fib[0]+Fib[1];
*/

 
Rancher
Posts: 42972
73
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to JavaRanch. What error are you getting? Post the stack trace.

I notice that you're creating a new Fibonacci object at each step - don't do that. One object (created in the main method) is sufficient. What is line 12 supposed to be doing? Remove that and see what happens.
 
Gabriel Camilleri
Greenhorn
Posts: 7
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Sorry for the late reply (did not realise how fast people here reply). Anyways I am using recursion (calling the same method) its a requirement. So that's the reason line 12 exists and that there are two objects. Also I tried it just in case but it still didn't work to your instructions. Thanks still
 
Ulf Dittmer
Rancher
Posts: 42972
73
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Recursion generally happens on the method level, not on the object level, so creating a new object at each step is not a hallmark of recursion. In fact, it would be unusual, certainly in this case.

The code looks OK to me if you were to remove lines 3 and 12. What happens if you do that, and then print the result of the call to Fib, instead of the value of pIndex?
 
Gabriel Camilleri
Greenhorn
Posts: 7
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I honestly do not know what's wrong. It's a logic error because I insert the index value of the fibonacci (i.e. the xth term) and I expect the result e.g. the fifth would be 3 (I consider the standard fibonacci meaning 0,1,1,2,3,5,8...). What I get instead is the same input I have given. (If you try it out you would see this problem). I personally feel even though the problem probably lies in line 3 & 12 those are the most important to create recursion & without it nothing would happen since the recursion loops.
 
Ulf Dittmer
Rancher
Posts: 42972
73
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What I get instead is the same input I have given.

That is because you're printing the input instead of the output:
Ulf Dittmer wrote:...and then print the result of the call to Fib, instead of the value of pIndex


I personally feel even though the problem probably lies in line 3 & 12 those are the most important to create recursion & without it nothing would happen since the recursion loops.

That's where your understanding is wrong. The recursion is in the Fib method, those objects contribute nothing and are unnecessary.
 
Gabriel Camilleri
Greenhorn
Posts: 7
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Still when i tried it w/o those lines the same thing happened. I also don't understand what you mean when you say I am not outputting the result.
 
lowercase baba
Bartender
Posts: 12565
49
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Here is your only output:
System.out.println("The value is: " + pIndex);

So the only thing you are printing is what pIndex holds. Looking at line 7, pIndex holds what the user gave you. The fact that you have a pIndex in your Fib method is irrelevant.

Also, what is the point of this:

If something is equal to zero, set it to zero. If it's equal to one, set it to one. That is 100% useless code, that accomplishes nothing. You would do better to re-write the if-block to better represent what you want to do.
 
Gabriel Camilleri
Greenhorn
Posts: 7
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well in the line 7 of the main method my understanding is that the user will say which cardinal number will be inserted. Meaning if the user gives 10 then it will be the tenth (contrary to what i said before though since i forgot to further explain there is the 0th index.) Also it wouldn't make much difference if there where those 2 if statements or if they were if(pIndex >= 2)...

That original format was how it was given so i tried to stick to that.
 
Ulf Dittmer
Rancher
Posts: 42972
73
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
That is 100% useless code, that accomplishes nothing.

Not quite useless - the assignments don't accomplish anything, but the if-tests prevent the recursion from being triggered when the parameter is 0 or 1. While having a single return statement in a method is generally good, in this case I would write it so that there are 3 return statements. That would avoid the need for any assignment in that method.
 
Gabriel Camilleri
Greenhorn
Posts: 7
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Do you mean like this? It would still be faulty :/ Do you think you can give me a snippet to show me directly (by code) how I'm wrong?
class Fibonacci{
public int Fib(int pIndex){
//Fibonacci f = new Fibonacci();
/*if((pIndex==0)||(pIndex == 1)){
return pIndex;
}*/
if(pIndex == 0){
pIndex = 0;
return pIndex;
}
else if(pIndex == 1){
pIndex = 1;
return pIndex;
}
else{
pIndex = (Fib(pIndex-2) + Fib(pIndex-1));
//f.Fib(pIndex);
return pIndex;
}

}
}
/*
Fib[5] = Fib[3]+Fib[4];
Fib[4] = Fib[2] + Fib[3];
Fib[3] = Fib[1]+Fib[2];
Fib[2] = Fib[0]+Fib[1];
Fib[3] = Fib[1] + Fib[2];
Fib[2] = Fib[0]+Fib[1];
*/
 
fred rosenberger
lowercase baba
Bartender
Posts: 12565
49
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ulf Dittmer wrote:
That is 100% useless code, that accomplishes nothing.

Not quite useless - the assignments don't accomplish anything, but the if-tests prevent the recursion from being triggered when the parameter is 0 or 1. While having a single return statement in a method is generally good, in this case I would write it so that there are 3 return statements. That would avoid the need for any assignment in that method.

Why not do this:

 
Ulf Dittmer
Rancher
Posts: 42972
73
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Gabriel Camilleri wrote:Do you mean like this? It would still be faulty

Is it? It looks OK to me. Even shorter would be "return 0;" and return 1;" - no assignment is necessary at all in the Fib method. Are you now printing out the returned value of the method call? All previous code snippets you showed just printed out the input value.

I'm beginning to think that you're not clear on how a method call returns its value, and how you would use it in the calling code. As Fred says, the fact that you have a variable "pIndex" variable in Fib that has the same name as some variable in the calling code is entirely irrelevant - the calling method has no access to that variable, so whatever you assign to it is lost anyway.
 
Gabriel Camilleri
Greenhorn
Posts: 7
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
fred rosenberger wrote:
Ulf Dittmer wrote:
That is 100% useless code, that accomplishes nothing.

Not quite useless - the assignments don't accomplish anything, but the if-tests prevent the recursion from being triggered when the parameter is 0 or 1. While having a single return statement in a method is generally good, in this case I would write it so that there are 3 return statements. That would avoid the need for any assignment in that method.

Why not do this:



I did what you said insert the index 10 and obviously re-inserted Line 3 then the run-time error of java.lang.StackOverflowError; came
 
Ulf Dittmer
Rancher
Posts: 42972
73
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Fred forgot to delete that line; as I said before, not only does it do nothing, it is actually harmful.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!