## alsz

hi everyone

I am having a little problem with the program below.
If I input "2/5 * 2/6" the char "/6" will be stored, so when I try to multiply by a simple variable by entering "3/7 * 6" the program will read "3/7 * 6/6".

is there a way to prevent this and if possible can you guys and or girls show me a work around?

P.S. the complete code is below. so if you want, you can copy and compile it to see what I mean.

`````` #include <iostream>
#include <cmath>
#include <cstdlib>

using namespace std;

class Rational
{
private:
int num;
int den;
public:

// Accessor and mutator functions
void setNumerator(int);
void setDenominator(int);
int getNumerator();
int getDenominator();
// Greatest common divisor, needed to simplify fractions
int gcd();
// Simplifies fractions
void simplify();
Rational operator*(Rational);
};

int main()

{
int option;
int ratNum1, ratNum2, ratDen1, ratDen2 = 0;
char ope1, ope2, ope3;
cout << "Rational numbers calculator " << endl;

do
{
cout << "1.To enter an operation\n";
cout << "2.To display the last result in decimal format\n";
cout << "Press 0 to quit.\n\n";
cout << "Please make a choice: ";
cin  >> option;
cout << "\n";

if (option == 1)
{
cout << "Please enter an operation: \n";
cin  >> ratNum1 >> ope1 >> ratDen1 >> ope2 >> ratNum2;
if (cin.peek() == '/')
{
cin >> ope3 >> ratDen2;
}
cout << ratNum1 << ope1 << ratDen1 << ope2 << ratNum2 << ope3 << ratDen2;
cout << "\n\n";

if (ope2 == '*' && ratDen2 == 0)
{
Rational rat1, rat2;

rat1.setNumerator(ratNum1);

rat1.setDenominator(ratDen1);

rat2.setNumerator(ratNum2);

rat2.setDenominator(ratDen2 = 1);
Rational rat3 = rat1 * rat2;

rat3.simplify();
cout << "The result of this operation is: " << rat3.getNumerator() << "/" << rat3.getDenominator() << " this is single" <<"\n\n";
}

if (ope2 == '*' && ratDen2 != 0 && ope3 == '/')
{
//delete[] ope3, *ratDen2;
Rational rat1, rat2;

rat1.setNumerator(ratNum1);

rat1.setDenominator(ratDen1);

rat2.setNumerator(ratNum2);

rat2.setDenominator(ratDen2);
Rational rat3 = rat1 * rat2;

rat3.simplify();
cout << "The result of this operation is: " << rat3.getNumerator() << "/" << rat3.getDenominator() << " this is rational"<< "\n\n";
}

}

if (option == 2)
{
if (ope2 == '*' && ratDen2 == 0)
{
Rational rat1, rat2;
rat1.setNumerator(ratNum1);

rat1.setDenominator(ratDen1);

rat2.setNumerator(ratNum2);

rat2.setDenominator(ratDen2=1);
Rational rat3 = rat1 * rat2;

rat3.simplify();
cout << "The result in decimal format is: "
<< (double(rat1.getNumerator()*rat2.getNumerator()))/(double(rat1.getDenominator()*1)) << "\n\n";
}

if (ope2 == '*' && ratDen2 != 0)
{
//delete[] ope3, *ratDen2;
Rational rat1, rat2;

rat1.setNumerator(ratNum1);

rat1.setDenominator(ratDen1);

rat2.setNumerator(ratNum2);

rat2.setDenominator(ratDen2);
Rational rat3 = rat1 * rat2;

rat3.simplify();
cout << "The result in decimal format is: "
<< (double(rat1.getNumerator()*rat2.getNumerator())/double(rat1.getDenominator()*rat2.getDenominator())) << "\n\n";
}

}

if (option != 1 && option != 2 && option != 0)
{
cout << "This is a wrong choice, please choose again... \n\n";
}

} while (option != 0);

cout << "Now quitting..." << endl;

system("PAUSE");
return 0;
}

//XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

void Rational::setNumerator(int n)
{
num = n;
}

void Rational::setDenominator(int n)
{
den = n;
}

int Rational::getNumerator()
{
return num;
}

int Rational::getDenominator()
{
return den;
}

// Greatest common divisor
int Rational::gcd()
{
int a = num;
int b = den;
int tmp;
// While b is not 0
while (b)
{
tmp = b;
b = a % b;
a = tmp;
}
return a;
}

// Simplifies the fraction
void Rational::simplify()
{
// Get the greatest common divisor
int gcdNum = gcd();
// If there is a common divisor, we don't want to divide by 0
if(gcdNum != 0)
{
// Set the new numerator
num = num / gcdNum;
// Set the new denominator
den = den / gcdNum;
}
}

Rational Rational::operator*(Rational ratPassed)
{
// Multiplying fractions
Rational ratResult;
ratResult.num = num * ratPassed.num;
ratResult.den = den * ratPassed.den;
return ratResult;
}
``````

This is a great start! You have clearly put some effort into it.

The easiest way to solve this problem with the inputs of the rational numbers is to create a separate function for inputting and outputting a rational number. And the best way to do this is to overload …

You have the class declaration, but did not the post the implementation. I'm assuming you simply copied and pasted his code verbatim.

However, I see don't see the word "friend" anywhere in your declaration and if you look at his code, you'll notice that it's this...

``istream& operator >> …``

The two functions that I posted (input and output) should be free functions (not members of the Rational class). You can simply insert them after the class declaration and before the main() function. Or, you can put them after the main function, but then, you have to insert the declarations …

Sorry about the "r" thing, it's a typo, it should be "rat" (i.e., the function parameter!). As in:

``````//output operator, to print a Rational number:
ostream& operator << (ostream& out, const Rational& rat) {
out << rat.getNumerator();
if(rat.getDenominator() != 1) // print denominator only if it is not 1.
out …``````

The two rational numbers, r1 and r2, should be declared outside of the loop if you want them to keep on storing the last values entered (just like you used to have those ratNum1, ratNum2.. variables before). So, if you move the declaration, `Rational r1, r2;` to where the "option" …

## VernonDozier 2,218

ratDen2 will be overwritten if and only if you read it in. If you don't it will remain whatever it was before you asked the user to enter an operation to simplify. The behavior is exactly as you describe. If you don't want to use the denominator of the LAST operation, you must overwrite it no matter what each time. If you think about it, 6 is the same as 6/1, so why not initialize ratDen2 to 1 each time prior to asking for user input? If the user inputs a fraction, ratDen2 will be overwritten, as desired. If not, you'll end up with 6 / 1 which is I imagine what you want.

## mike_2000_17 2,669

This is a great start! You have clearly put some effort into it.

The easiest way to solve this problem with the inputs of the rational numbers is to create a separate function for inputting and outputting a rational number. And the best way to do this is to overload the `<<` and `>>` operators for the iostreams. By simply taking your code and putting it into the following functions:

``````//output operator, to print a Rational number:
ostream& operator << (ostream& out, const Rational& rat) {
out << r.getNumerator();
if(r.getDenominator() != 1)  // print denominator only if it is not 1.
out << "/" << r.getDenominator();
return out; // don't forget to return the ostream object.
};

//input operator, to input a Rational number from a stream:
istream& operator >> (istream& in, Rational& rat) {   // notice the & here.
int num = 0, den = 1;
in >> num;
if(in.peek() == '/') {  //check if there is a denominator.
char temp;
in >> temp >> den; // read the denominator.
};
r.setNumerator(num);
r.setDenominator(den);
return in; // don't forget to return the istream object.
};
``````

Notice that I used pass-by-reference in the above. In the first case, I pass a const-reference, which means that the variable (within the function) refers directly to the variable on which it was called, but without permission to change it. In the second case (input), I pass by reference (non-const), which means that there is permission to change the variable, and because it refers to the variable that was passed to that function (or operator), the changes will take effect on that variable.

In order to make the above work, there is one change you need to make to your Rational class code. That is, you need to make the functions `getNumerator()` and `getDenominator()`const` functions too. You need to write this on the declaration:

``````int getNumerator() const;   //notice the const here.
int getDenominator() const; //notice the const here.
``````

And, you will need to add that `const` keyword in your definitions too (or implementations of the functions).

Now, with this, your main function can be much simplified. As an example, I can just show the equivalent of your code for the "option 1" part, as so:

``````     cout << "Please enter an operation: \n";
Rational r1, r2; // declare too rational numbers;

cin >> r1 >> ope2 >> r2; // take the input (will automatically handle any case).

cout << r1 << ope2 << r2; // print it out again.
cout << "\n\n";

if( ope2 == '*' ) {   // check for multiplication.
Rational r3 = r1 * r2;  // perform the multiplication.
r3.simplify();          // simplify. and print results:
cout << "The result of this operator is: " << r3 << " this is " << (r3.getDenominator() == 1 ? "an integer." : "a rational number.") << "\n\n";
};
``````

Simpler this way, isn't it? This is just thanks to putting the input/output code in separate functions (and having them as operator overloads on iostreams is nice and fancy, but they could also be simple `print()` and `getInput()` functions in the Rational class).

commented: @Mike O_o I am so sorry , but how do you exactly implement the first part of your solution into my code or my code into your solution. I have never seen the istream and ostream. Do I have to declare them in the class ? +0

## alsz

@Mike O_o I am so sorry , but how do you exactly implement the first part of your solution into my code or my code into your solution.

I have never seen the istream and ostream. Do I have to declare them in the class ?

## alsz

so for the functions do I have to declare them in the class or is it applied directly into the body of the code also when I declared it in the class I got a compiler error saying

" ```std::ostream& Rational::operator<<(std::ostream&, const Rational&)' must take exactly one argument" and "```std::istream& Rational::operator>>(std::istream&, Rational&)' must take exactly one argument "

``````    class Rational
{
private:
int num;
int den;
public:

// Accessor and mutator functions
void setNumerator(int);
void setDenominator(int);
int getNumerator();
int getDenominator();
// Greatest common divisor, needed to simplify fractions
int gcd();
// Simplifies fractions
void simplify();
Rational operator*(Rational);
ostream& operator << (ostream& out, const Rational& rat);
istream& operator >> (istream& in, Rational& rat);
};
``````

## VernonDozier 2,218

You have the class declaration, but did not the post the implementation. I'm assuming you simply copied and pasted his code verbatim.

However, I see don't see the word "friend" anywhere in your declaration and if you look at his code, you'll notice that it's this...

``````istream& operator >> (istream& in, Rational& rat)
``````

Note that there is no `Rational::` in his code. Is it in yours? It should not be. The error message suggests you are making his code a member function of the Rational class rather than just a friend.

So stick the word "friend" at the beginning of lines 18 and 19, then copy his code into your code.

If "friend functions" are something you have not seen before, here's an example...

## mike_2000_17 2,669

The two functions that I posted (input and output) should be free functions (not members of the Rational class). You can simply insert them after the class declaration and before the main() function. Or, you can put them after the main function, but then, you have to insert the declarations between the Rational class declaration and the main function, as so:

``````class Rational {
//..
};

ostream& operator << (ostream& out, const Rational& rat);  // declaration.
istream& operator >> (istream& in, Rational& rat);         // declaration.

int main() {
//... ...
};

// then, somewhere here, insert the code I gave, verbatim.
``````

The error message you got is because those functions should not be member functions of the Rational class, they must be free functions (outside the class, so, no `Rational::` should appear).

And, remember to make the two get-functions `const` as I said earlier. Otherwise, you'll get an error like this:

``````error: passing ‘const Rational’ as ‘this’ argument of ‘int Rational::getNumerator()’ discards qualifiers [-fpermissive]
``````

## alsz

ok I will give it another shot

## alsz

Hi mike:

ok so I copy everything verbatim this time and the program said " r " was undefined, so I defined it by using "Rational r" so it could compile. so when I enter something like "2/5*3/6" the program is reading something crazy like "3/6 * 3/16384". Lastly, for the functions;

``````ostream& operator << (ostream& out, const Rational& rat); // declaration.
istream& operator >> (istream& in, Rational& rat); // declaration.
``````

do I have to declare it in the main? or it works just like my " operator* " function?

Best regards, alsz

``````#include <iostream>
#include <cmath>
#include <cstdlib>
using namespace std;
class Rational
{
private:
int num;
int den;
public:
// Accessor and mutator functions
void setNumerator(int);
void setDenominator(int);
int getNumerator()const;    //const
int getDenominator()const;  //as you told me
// Greatest common divisor, needed to simplify fractions
int gcd();
// Simplifies fractions
void simplify();
Rational operator*(Rational);
};

ostream& operator << (ostream& out, const Rational& rat); // declaration.
istream& operator >> (istream& in, Rational& rat); // declaration.

int main()
{
int option;
int ratNum1, ratNum2, ratDen1, ratDen2 = 0;
char ope1, ope2, ope3;
cout << "Rational numbers calculator " << endl;
do
{
cout << "1.To enter an operation\n";
cout << "2.To display the last result in decimal format\n";
cout << "Press 0 to quit.\n\n";
cout << "Please make a choice: ";
cin >> option;
cout << "\n";
if (option == 1)
{

//**************************************************
//verbatim

cout << "Please enter an operation: \n";
Rational r1, r2; // declare too rational numbers;
cin >> r1 >> ope2 >> r2; // take the input (will automatically handle any case).
cout << r1 << ope2 << r2; // print it out again.
cout << "\n\n";
if( ope2 == '*' ) { // check for multiplication.
Rational r3 = r1 * r2; // perform the multiplication.
r3.simplify(); // simplify. and print results:
cout << "The result of this operator is: " << r3 << " this is " << (r3.getDenominator() == 1 ? "an integer." : "a rational number.") << "\n\n";
};
if (ope2 == '*' && ratDen2 != 0 && ope3 == '/')
{
//delete[] ope3, *ratDen2;
Rational rat1, rat2;
rat1.setNumerator(ratNum1);
rat1.setDenominator(ratDen1);
rat2.setNumerator(ratNum2);
rat2.setDenominator(ratDen2);
Rational rat3 = rat1 * rat2;
rat3.simplify();
cout << "The result of this operation is: " << rat3.getNumerator() << "/" << rat3.getDenominator() << " this is rational"<< "\n\n";
}
}
if (option == 2)
{
if (ope2 == '*' && ratDen2 == 0)
{
Rational rat1, rat2;
rat1.setNumerator(ratNum1);
rat1.setDenominator(ratDen1);
rat2.setNumerator(ratNum2);
rat2.setDenominator(ratDen2=1);
Rational rat3 = rat1 * rat2;
rat3.simplify();
cout << "The result in decimal format is: "
<< (double(rat1.getNumerator()*rat2.getNumerator()))/(double(rat1.getDenominator()*1)) << "\n\n";
}
if (ope2 == '*' && ratDen2 != 0)
{
//delete[] ope3, *ratDen2;
Rational rat1, rat2;
rat1.setNumerator(ratNum1);
rat1.setDenominator(ratDen1);
rat2.setNumerator(ratNum2);
rat2.setDenominator(ratDen2);
Rational rat3 = rat1 * rat2;
rat3.simplify();
cout << "The result in decimal format is: "
<< (double(rat1.getNumerator()*rat2.getNumerator())/double(rat1.getDenominator()*rat2.getDenominator())) << "\n\n";
}
}
if (option != 1 && option != 2 && option != 0)
{
cout << "This is a wrong choice, please choose again... \n\n";
}
} while (option != 0);
cout << "Now quitting..." << endl;
system("PAUSE");
return 0;
}
//XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
void Rational::setNumerator(int n)
{
num = n;
}
void Rational::setDenominator(int n)
{
den = n;
}
int Rational::getNumerator()const   //const as you told me
{
return num;
}
int Rational::getDenominator()const  // same
{
return den;
}
// Greatest common divisor
int Rational::gcd()
{
int a = num;
int b = den;
int tmp;
// While b is not 0
while (b)
{
tmp = b;
b = a % b;
a = tmp;
}
return a;
}
// Simplifies the fraction
void Rational::simplify()
{
// Get the greatest common divisor
int gcdNum = gcd();
// If there is a common divisor, we don't want to divide by 0
if(gcdNum != 0)
{
// Set the new numerator
num = num / gcdNum;
// Set the new denominator
den = den / gcdNum;
}
}
Rational Rational::operator*(Rational ratPassed)
{
// Multiplying fractions
Rational ratResult;
ratResult.num = num * ratPassed.num;
ratResult.den = den * ratPassed.den;
return ratResult;
}

//*******************************************************************
//declared Rational r so program can compile

//output operator, to print a Rational number:
ostream& operator << (ostream& out, const Rational& rat) {
Rational r;
out << r.getNumerator();
if(r.getDenominator() != 1) // print denominator only if it is not 1.
out << "/" << r.getDenominator();
return out; // don't forget to return the ostream object.
};

//input operator, to input a Rational number from a stream:
istream& operator >> (istream& in, Rational& rat) { // notice the & here.
int num = 0, den = 1;
Rational r;
in >> num;
if(in.peek() == '/') { //check if there is a denominator.
char temp;
in >> temp >> den; // read the denominator.
};
r.setNumerator(num);
r.setDenominator(den);
return in; // don't forget to return the istream object.
};
``````

## mike_2000_17 2,669

Sorry about the "r" thing, it's a typo, it should be "rat" (i.e., the function parameter!). As in:

``````//output operator, to print a Rational number:
ostream& operator << (ostream& out, const Rational& rat) {
out << rat.getNumerator();
if(rat.getDenominator() != 1) // print denominator only if it is not 1.
out << "/" << rat.getDenominator();
return out; // don't forget to return the ostream object.
};
//input operator, to input a Rational number from a stream:
istream& operator >> (istream& in, Rational& rat) { // notice the & here.
int num = 0, den = 1;
in >> num;
if(in.peek() == '/') { //check if there is a denominator.
char temp;
in >> temp >> den; // read the denominator.
};
rat.setNumerator(num);
rat.setDenominator(den);
return in; // don't forget to return the istream object.
};
``````
commented: Hey Mike, No need to apologize man, I should have figured it out myself. I really appreciate the help and thank you so much for all the help and hints. Have an awesome day!! Sincerely, alsz +0

## alsz

Hey Mike,

No need to apologize man, I should have figured it out myself. I really appreciate the help and thank you so much for all the help and hints.

Have an awesome day!!
Sincerely, alsz

@Vernon thanks too.

## alsz

now I am encountering another problem... the program is simply not storing the varibles mike or anyone else that can help so when the option 2 is selected the franction from before is not being read .... instead it is random numbers.

why is this happening?

## VernonDozier 2,218

Not sure what the updated code is,but I see integers ratNum1, ratNum2, ratDen1, ratDen2 in there still. The whole point of writing the << and >> operators was so you could get rid of those. Turn on your compiler warnings and see if you get any "unitinitialized variable" warnings. Line 57 looks like it should get one. Or perhaps not. I can't tell what is inside of what bracket due to the lack of indentation in your code.

Anyway, it looks like you partially upgraded your code to use the >> and << operators, but you still have some relics of your old code in there that use ratNum1, ratNum2, ratDen1, ratDen2. That's actually the worst of both worlds because unlike before, they don't appear to ever get anything read into them. I imagine that's why everything appears "random".

## alsz

Hi Vernon here is my updated code.

I basically followed mike's suggestions and codes, but unlike my original code, his modifications does not permanently store the inputted variables and I don't know why.

oo and my compiler (dev c++) does not warn me of anything about "unitialized variable".

``````#include <iostream>
#include <cmath>
#include <cstdlib>

using namespace std;

class Rational
{
private:
int numerator;
int denominator;

public:
// Accessor and mutator functions
void setNumerator(int);
void setDenominator(int);
int getNumerator()const;
int getDenominator()const;
// greatest common divisor, needed to simplify fractions
int gcd();
// Simplifies fractions
void reduce();
Rational operator+(Rational);
Rational operator*(Rational);
};

ostream& operator << (ostream& out, const Rational& rat); // declaration.
istream& operator >> (istream& in, Rational& rat); // declaration.

int main()
{
int option;
char ope2 = ' ';
cout << "Rational numbers calculator " << endl;
do
{
cout << "1.To enter an operation\n";
cout << "2.To display the last result in decimal format\n";
cout << "Press 0 to quit.\n\n";
cout << "Please make a choice: ";
cin >> option;
cout << "\n";

if (option == 1)
{
cout << "Please enter an operation: \n";
Rational r1, r2; // declare too rational numbers;
cin >> r1 >> ope2 >> r2; // take the input (will automatically handle any case).
//cout << r1 << ope2 << r2; // print to see if values were read correctly
cout << "\n";
if (ope2 == '+')
{
Rational r3 = r1 + r2; // perform the multiplication.
r3.reduce(); // reduce results:
cout << "The result of this operator is: " << r3 << "\n\n";
}

if( ope2 == '*' )
{ // check for multiplication.
Rational r3 = r1 * r2; // perform the multiplication.
r3.reduce(); // reduce results:
cout << "The result of this operator is: " << r3 << "\n\n";
}
}

if (option == 2)
{
Rational r1, r2; // declare too rational numbers;

if( ope2 == '+' )
{ // check for multiplication.
Rational r3 = r1 + r2; // perform the multiplication.
r3.reduce(); // reduce results:
cout << "The result of this operator is: " << (double(r3.getNumerator())/double(r3.getDenominator())) << "\n\n";
}

if( ope2 == '*' )
{ // check for multiplication.
Rational r3 = r1 * r2; // perform the multiplication.
r3.reduce(); // reduce results:
cout << "The result of this operator is: " << (double(r3.getNumerator())/double(r3.getDenominator())) << "\n\n";
}
}

if (option != 1 && option != 2 && option != 0)
{
cout << "This is a wrong choice, please choose again... \n\n";
}

} while (option != 0);
cout << "Now quitting..." << endl;

system("PAUSE");
return 0;
}

void Rational::setNumerator(int num)
{
numerator = num;
}

void Rational::setDenominator(int den)
{
denominator = den;
}

int Rational::getNumerator()const
{
return numerator;
}

int Rational::getDenominator()const
{
return denominator;
}

// Greatest common divisor
int Rational::gcd()
{
int a = numerator;
int b = denominator;
int tmp;
// While b is not 0
while (b)
{
tmp = b;
b = a % b;
a = tmp;
}
return a;
}

// reduce the fraction
void Rational::reduce()
{
// get the greatest common divisor
int gcdNum = gcd();
// If there is a common divisor, we don't want to divide by 0
if(gcdNum != 0)
{
// Set the new numerator
numerator = numerator / gcdNum;
// Set the new denominator
denominator = denominator / gcdNum;
}
}

Rational Rational::operator+(Rational ratPassed)
{
Rational ratResult;
ratResult.numerator = numerator * ratPassed.denominator + denominator * ratPassed.numerator;
ratResult.denominator = denominator * ratPassed.denominator;
return ratResult;
}

Rational Rational::operator*(Rational ratPassed)
{
// Multiplying fractions
Rational ratResult;
ratResult.numerator = numerator * ratPassed.numerator;
ratResult.denominator = denominator * ratPassed.denominator;
return ratResult;
}

//output operator, to print a Rational number:
ostream& operator << (ostream& out, const Rational& rat)
{
out << rat.getNumerator();
if(rat.getDenominator() != 1) // print denominator only if it is not 1.
out << "/" << rat.getDenominator();
return out; //return the ostream object.
};

//input operator, to input a Rational number from a stream:
istream& operator >> (istream& in, Rational& rat)
{
int numerator = 0, denominator = 1;
in >> numerator;
if(in.peek() == '/')  //check if there is a denominator.
{
char temp;
in >> temp >> denominator; // read the denominator.
};
rat.setNumerator(numerator);
rat.setDenominator(denominator);
return in; //return the istream object.
};
``````

## alsz

Basically everything about the calculations runs perfectly, but the thing is that when I use option 2 to turn the last entered fraction result, it does not give me the right answer or it will be something like 1/infinity

## mike_2000_17 2,669

The two rational numbers, r1 and r2, should be declared outside of the loop if you want them to keep on storing the last values entered (just like you used to have those ratNum1, ratNum2.. variables before). So, if you move the declaration, `Rational r1, r2;` to where the "option" variable is declared, all should work fine.

## VernonDozier 2,218

oo and my compiler (dev c++) does not warn me of anything about "unitialized variable".

I was looking at your old code when I made that comment. You've taken out those old variables (which is good, and again which was the goal with the >> and << operators), so you can ignore that comment.

Adding to Mike's comment, note that you have r1 and r2 declared on both lines 48 and 69. If you declare them at line 32 as Mike suggested, you'll want to not only delete line 69, but you'll also need to delete line 48. If you merely move line 69 to line 32, the r1 and r2 variables on line 32 will never be filled in on line 49 dues to scoping issues (line 49 will use the r1 and r2 with the most local scope). So again, make sure you have one and only one declaration of r1 and r2 and make sure it's at the top so that 1) it does not go out of scope, and 2) it will get filled in.