PDA

View Full Version : what's wrong with this code?




dajar
Aug 27, 2006, 04:02 PM
I was solving an example to improve my C++, though I couldn't solve it the way it's meant to be. The problem says:

Write a recursive function that will compute the sum of the first n integers in an array of at least n integers. Hint: begin with nth integer.

here's my solution:

#include <iostream>
#include <string>
#include <vector>

using namespace std;

int temp;

int integerSummer(vector<int> new_one, int n)
{
if(n == 0)
return 0;

else
temp = new_one[n];
return temp + integerSummer(new_one, n - 1);

}

int main()
{
vector<int> new_one;

new_one[0] = 11;
new_one[1] = 20;
new_one[2] = 21;
new_one[3] = 17;
new_one[4] = 5;

integerSummer(new_one, 3);
system("pause");
return 0;
}




Program exits immediately after executing it. What's wrong with the code? Any ideas?:confused:



kainjow
Aug 27, 2006, 04:29 PM
Well for one thing, "pause" doesn't exist on Mac OS X so when you use system("pause"); it doesn't pause the program. Try using something like this instead: char a;
cin >> a;

mkrishnan
Aug 27, 2006, 04:32 PM
Maybe I'm saying something obvious... but your code doesn't really do much of anything. So how do you know it isn't just doing it and exiting happily? Main just runs the integerSummer and doesn't even do anything with the result. It doesn't take very long to calculate the result. So it seems like nothing happened. You should start by at least outputting / returning the result! ;)

sherpa78
Aug 27, 2006, 04:39 PM
Well, for starters, are you outputting anything to the screen? Your program does the function calls, pauses, and then ends. No output to the screen at all.

Your recursive function, integerSummer, takes two arguments: a vector and an index into that vector. From your call, integerSummer(new_one, 3), I'm guessing you want to calculate the sum of the first 4 numbers: elements 3, 2, 1, 0?

If so, there's a slight problem: element 0 will always be zero. In your new_one vector, however, element 0 is 11. This is what the sum would look like: 17 + 21 + 20 + 0. That last number should have been an 11.

Good luck finding the right solution. Recursive solutions are fun! :)

Sherpa78

Sayer
Aug 27, 2006, 06:10 PM
Also, the code only sums the nth and n - 1 value of the vector. You need to loop over the vector and add them up (with a running total) n times.

I find it amusing that a lot of CS classes require programming experience in order for the student to learn programming. Go figure.

mkrishnan
Aug 27, 2006, 06:40 PM
Also, the code only sums the nth and n - 1 value of the vector. You need to loop over the vector and add them up (with a running total) n times.

I might be dense and really I don't even know why I'm looking at code today... :p But I think this is not true. The core recursion idea is correct here -- main calls integerSummer at position 3. That call in turn calls integerSummer at position 2. That call in turn calls integerSummer at position 1. And then the last call returns a discrete value (0) which allows for the recursion to close and allows the right-hand-side value in this line calculated:

return temp + integerSummer(new_one, n - 1);

The issue of summing position zero aside, I think the recursion works....

MrFrankly
Aug 27, 2006, 06:47 PM
Don't have much to add to the suggestions from the other people here. If you follow their advice your program will work. I however noticed this in your code:


if(n == 0)
return 0;

else
temp = new_one[n];
return temp + integerSummer(new_one, n - 1);


You're applying an if statement without using brackets. It seems you're trying to say this


if(n ==0) {
return 0;
} else {
temp = new_one[n];
return temp + integerSummer(new_one, n - 1);
}


however, because you're not using the brackets, the code will be read by the compiler as follows.


if(n ==0) {
return 0;
} else {
temp = new_one[n];
}

return temp + integerSummer(new_one, n - 1);


Funny enough, in this case it actually works correct because you're exiting from the function on time in the n==0 case because of the return. But someday some case will pop up where it doesn't work correctly and it will take ages before you find the problem. I would recommend to always use brackets where you can, it makes the code much more clear to read and it avoids bugs.

NewbieNerd
Aug 29, 2006, 08:40 AM
So your problem says to sum up the first n numbers of an array. Remember that indexing in an array starts at 0. So if someone wanted the sum where n=1, they would just want the first element of the array, namely new_one[0]. If n=2, the would want the sum of the first TWO elements, namely new_one[0] + new_one[1]. So in your recursion, you should say that temp = new_one[n-1], then recurse with integerSummer(new_one, n-1). That will work.