Hey guys, theres a problem with the loop inside the yesno function here.. it kinda of hmm.. keeps the first number i enter and gives me the factorial of that number even if i enter another number the second time.. also my no option still gives me the loop even though I put a return 0; (?)

I will really appreciate any help I can get.. I've been struggling with this and its due tomorrow at noon >.<

Thanks in advance.

#include <iostream>

using namespace std;

int yesno();
double factorial(double num); //factorial prototype

int main ()
{
	int number;
	char choice;
	cout << "Please enter a number: ";
	cin >> number;
	
	while(number > 0)
	{
		int x = factorial(number);
		cout << "Factorial = " <<x <<endl;
		cout << "Would you like to go again?";
		cin >> choice;

		if (choice == 'n' || choice == 'N')
		{ cout <<"Thank you for using this program." << "\n";}
		else if (choice == 'y' || choice == 'Y')
		{ 
			char choice = yesno();
		}
		else {cout << "Pay attention and start again" <<"\n";}
	}
	return 0;
system ("pause");
}

//yesno
int yesno()
{
	do{
		int number;
	char choice;
	cout << "Please enter a number: ";
	cin >> number;
	
	while(number > 0)
	{
		int x = factorial(number);
		cout << "Factorial = " <<x <<endl;
		cout << "Would you like to go again?";
		cin >> choice;

		if (choice == 'n' || choice == 'N')
		{ 
			cout <<"Thank you for using this program." << "\n"; 
		return 0;
		}
		else if (choice == 'y' || choice == 'Y')
		{ 
			char choice = yesno();
			return 1;
		}
		else 
		{
			cout << "Pay attention and start again" <<"\n";
		}
	}
	} while (yesno() == 1);
}
//compute factorial 
double factorial (double num)
{
	int fac=1;
	for(int m=1; m<=num; num--) 
      fac = fac * num;
	return fac;
}

Since your main() and your yesno() function appear to do almost exactly the same thing, you probably should spend a little time re-thinking your whole program, as short as it is.

If you were to ask me (not that you did), I'd say a function called yesno should only ask the user if he wants to run the program again, and return true if he does, false if he doesn't and loop until his response is valid. Meaning its return type should be bool rather than int, and should probably be called something more specific, like runAgain(). Then you could do something like (this is pseudocode, but making it C++ is not hard):

keep doing {
   get a number from the user
   compute factorial
   display result
} while runAgain() returns true

Your code will be much smaller and more manageable.

That said, yesno() currenty returns an int (0 or 1), but in main() you not only assign it to choice (a variable of type char), and then don't look at that value of choice once you've returned, but the variable is in fact local to that else-if block, since you redeclare it there (specifying "char" again makes a new local variable that masks the one in the outside scope).

Then inside yesno(), you have that function call itself twice. Once in the same else-if block, again assigning the int result of the recursive call to a local char variable which you then ignore and just return 1. And again to determine if your while loop should continue. Instead of trying to write all of this in one pass of bashing it with a big stick and hoping it will eventually do what you want, try writing just a little bit of it at a time, and get each little bit to work, and then think logically about how to fit the bits together into bigger bits and an entire program. If you can clearly and precisely say what the program should do, it becomes easy to write the code to do it.

Edited 5 Years Ago by raptr_dflo: n/a

This article has been dead for over six months. Start a new discussion instead.