# What's wrong with this code?

This is a discussion on What's wrong with this code? within the C++ Programming forums, part of the General Programming Boards category; I need some help. What's wrong with the code below? Code: const int i = 500; int y, n = ...

1. ## What's wrong with this code?

I need some help. What's wrong with the code below?

Code:
```const int i = 500;
int y, n = 0, z = 0;
double r, a;

cout << "Inset amount you want to borrow ";
cin   >> a;
cout << "Do you want the 1, 3 or 10 year repayment plan? ";
cin   >> y;

if (y = 1)
{
y = 1;
r = 8.5;
}
else if (y = 3)
{
y = 3;
r = 9;
}
else if (y = 10)
{
y = 10;
r = 10;
}
else
{
cout << "wrong input!" << endl;
y = 0;
//how do I make it jump back up to the first cin?
}

while (n < y)
{
a += a/100*r;
z += a;
cout << "You'll pay a monthly rate of " << (a + i)/12 << " credits" << endl
<< "You will have payed " << z + i << " credits in all";
n++;
}```
And is there a way I can make the else statement jump back to the first cin in the code?

PS: I'm a total n00b, so bare with me.

2. You just need to use a loop. Like so
Code:
```int correctInput = 0;
while(correctInput==0)
{
cout << "Inset amount you want to borrow ";
cin   >> a;
cout << "Do you want the 1, 3 or 10 year repayment plan? ";
cin   >> y;

if (y = 1)
{
y = 1;
r = 8.5;
correctInput = 1;//This will exit the loop
}
else if (y = 3)
{
y = 3;
r = 9;
correctInput = 1;
}
else if (y = 10)
{
y = 10;
r = 10;
correctInput = 1;
}
else
{
cout << "wrong input!" << endl;
y = 0;
//how do I make it jump back up to the first cin?
}
}```

3. That makes sense, thanks.

Could you also tell me why the code only works for 1 year?

4. Woops didn't even notice that.
Code:
`if(y=1)`
should be
Code:
`if(y==1)`
So on and so forth.

5. because you put y = 3 which will always return true (barring any serious computer malfunctions)....... use 2 "==" like so y == 3

6. sorry, nevermind.

7. It's working !

Hehe, thanks for your help, I'll be sure to use it in the future

Code:
```{
const int i = 500;
int n = 0, y = 0, b = 0;
double r, a, z = 0;

cout << "---------------------Banky-bank loans--------------------" << endl
<< "We offer loans at only 8.5, 9 or 10%, \ndepending on how long the repayment period will be."
<< endl << "---------------------------------------------------------"
<< endl << "We charge a minor 500 credit administration fee."
<< endl << "---------------------------------------------------------"
<< fixed << setprecision(2);

while (b == 0)
{
cout << endl << endl << "Inset amount you want to borrow ";
cin  >> a;
cout << "Do you want the 1, 3 or 10 year repayment plan? ";
cin  >> y;
b = 1;

if (y == 1)
{
r = 8.5;
while (n < y)
{
a += (a/100)*r;
n++;
}
z += a;
cout << "You'll pay a monthly rate of " << (i+a)/12 << " credits" << endl
<< "You will have payed " << z+i << " credits in all" << endl;
}
else if (y == 3)
{
r = 9;
while (n < y)
{
a += (a/100)*r;
n++;
}
z += a;
cout << "You'll pay a monthly rate of " << (i+a)/36 << " credits" << endl
<< "You will have payed " << z+i << " credits in all" << endl;
}
else if (y == 10)
{
r = 10;
while (n < y)
{
a += (a/100)*r;
n++;
}
z += a;
cout << "You'll pay a monthly rate of " << (i+a)/120 << " credits" << endl
<< "You will have payed " << z+i << " credits in all" << endl;
}
else
{
cout << "Wrong input! " << endl;
b = 0;
}
}```
Any ideas how this could have been made simpler?

8. a little piece of advice.....use the tab key liberally....it will save you much heartache

Code:
```if()
{
int i = 2;
while()
{
int x = 65;
if()
{
int junk = 83
for()
{
//see how much easier it is to tell
//where something ends and another
//begins
}
//this
}
//will help
}
//with debugging
}

//logic errors```

9. Like so?

Code:
```{
const int i = 500;
int n = 0, y = 0, b = 0;
double r, a, z = 0;

cout << "---------------------Banky-bank loans--------------------" << endl
<< "We offer loans at only 8.5, 9 or 10%, \ndepending on how long the repayment period will be."
<< endl << "---------------------------------------------------------"
<< endl << "We charge a minor 500 credit administration fee."
<< endl << "---------------------------------------------------------"
<< fixed << setprecision(2);

while (b == 0)
{
cout << endl << endl << "Inset amount you want to borrow ";
cin  >> a;
cout << "Do you want the 1, 3 or 10 year repayment plan? ";
cin  >> y;
b = 1;

if (y == 1)
{
r = 8.5;
while (n < y)
{
a += (a/100)*r;
n++;
}
z += a;
cout << "You'll pay a monthly rate of " << (i+a)/12 << " credits" << endl
<< "You will have payed " << z+i << " credits in all" << endl;
}
else if (y == 3)
{
r = 9;
while (n < y)
{
a += (a/100)*r;
n++;
}
z += a;
cout << "You'll pay a monthly rate of " << (i+a)/36 << " credits" << endl
<< "You will have payed " << z+i << " credits in all" << endl;
}
else if (y == 10)
{
r = 10;
while (n < y)
{
a += (a/100)*r;
n++;
}
z += a;
cout << "You'll pay a monthly rate of " << (i+a)/120 << " credits" << endl
<< "You will have payed " << z+i << " credits in all" << endl;
}
else
{
cout << "Wrong input! " << endl;
b = 0;
}
}
cin  >> b;
}```

10. As I look at your code the following sequence is repeated three times:
Code:
```while (n < y)
{
a += (a/100)*r;
n++;
}
z += a;
cout << "You'll pay a monthly rate of " << (i+a)/12 << " credits" << endl << "You will have payed " << z+i << " credits in all" << endl;```
You could wrap that sequence in a function and call the function three times rather than rewriting all that out three times. In addition I can't see a need for z at all, just use a wherever you have z.

11. But I need it to devide by 12, 36 and 120 months, so they aren't identical. How would you suggest I'd solve that?

You were right about the z variable. Thanks.

Is this what you had in mind?

Code:
```#include <iostream>
#include <iomanip>
#pragma hdrstop
using namespace std;

void ball(int g, int i, double a)
{
cout << fixed << setprecision(2) << endl
<< "You'll pay a monthly rate of " << (i+a)/g << " credits" << endl
<< "You will have payed " << a+i << " credits in all" << endl;
}

//---------------------------------------------------------------------------

#pragma argsused
main()
{
const int i = 500;
int n = 0, y = 0, b = 0;
double r, a;

cout <<         "---------------------Banky-bank loans--------------------"
<< endl << "We offer loans at only 8.5, 9 or 10%, "
<< endl << "depending on how long the repayment period will be."
<< endl << "---------------------------------------------------------"
<< endl << "We charge a minor 500 credit administration fee."
<< endl << "---------------------------------------------------------";

while (b == 0)
{
cout << endl << endl << "Inset amount you want to borrow ";
cin  >> a;
cout << endl << "Do you want the 1, 3 or 10 year repayment plan? ";
cin  >> y;
b = 1;

if (y == 1)
{
r = 8.5;
while (n < y)
{
a += (a/100)*r;
n++;
}
ball(12, i, a);
}
else if (y == 3)
{
r = 9;
while (n < y)
{
a += (a/100)*r;
n++;
}
ball(36, i, a);
}
else if (y == 10)
{
r = 10;
while (n < y)
{
a += (a/100)*r;
n++;
}
ball(120, i, a);
}
else
{
cout << "Wrong input! " << endl;
b = 0;
}
}
cin  >> b;
}```

12. A few other things to help clarify the program
1. All the computations are enclosed in a loop, but the purpose of the loop is not to repeat the computations. The only reason for the loop is to make sure the user puts in correct input.

So move all the computations AFTER the loop, like this:
Code:
```// banky-bank advertisement here
// ask for amount to borrow (variable a)
// now ask for variable y as follows:

y = -1;		//deliberately set y to something invalid

while ( ! (y==1 || y==3 || y==10) ) {	// while y is invalid
}

//NOW do the calculations AFTER the input loop.```
2. with the ifs, the only thing that is different for the different y's is the r and the number of months (make another variable for the months). The (while (n<y)) loop is the SAME for all the different y's, so it's logically clearer (and shorter) to take it out of the ifs, instead of repeating it over and over. So,

Code:
```if (y==1) {
r = 8.5;
months = 12;
}
else if (y==3) {
r = 9;
months = 36;
}
else if (y==10) {
r = 10;
months = 120;
}

//NOW do the while (n<y) to calculate a, and output monthly rate using ball(months, i, a).```

13. Code:
```if (y==1) {
r = 8.5;
ball(y, r);
}
else if (y==3) {
r = 9;
ball(y, r);
}
else if (y==10) {
r = 10; ;
ball(y, r);
}
else{
cout << "Wrong input! " << endl;
b = 0;
}
---------------------------------------
void ball(int year, int rate)
{
int months = year * 12;
int n = 0;
int a = 0;
const int i = 500;

while (n < year)
{
a += (a/100)*rate;
n++;
}

cout << "You'll pay a monthly rate of " << (i+a)/months << " credits" << endl;
cout << "You will have payed " << a+i << " credits in all" << endl;
}```

14. That's a great observation--you don't even need a month variable, since you can just use 12*y for the number of months. That makes the "if" part even simpler.

If you put all the output at the end, then you don't need to make a function to do it, because it's short and not repeated over and over.

Code:
```#include <iostream>
#include <iomanip>
#pragma hdrstop
using namespace std;

main()
{
#pragma argsused
const int i = 500;
int n = 0, y = 0;
double r, a;

cout <<         "---------------------Banky-bank loans--------------------"
<< endl << "We offer loans at only 8.5, 9 or 10%, "
<< endl << "depending on how long the repayment period will be."
<< endl << "---------------------------------------------------------"
<< endl << "We charge a minor 500 credit administration fee."
<< endl << "---------------------------------------------------------";

cout << endl << endl << "Inset amount you want to borrow ";
cin  >> a;

while ( ! (y==1 || y==3 || y==10) ) {
cout << endl << "Do you want the 1, 3 or 10 year repayment plan? ";
cin  >> y;
}

if (y==1) r = 8.5;
else if (y==3) r = 9;
else /* y==10 */ r = 10;

while (n < y) {
a += (a/100)*r;
n++;
}

cout << fixed << setprecision(2) << endl
<< "You'll pay a monthly rate of " << (i+a)/(12*y) << " credits" << endl
<< "You will have payed " << a+i << " credits in all" << endl;

}```

15. about the indention and {}, you're close...line up the { under the first letter of the control structure (loops and if's) or at the end of the line

Code:
```if()
{
//indent this part
a = b;
}

or

if(){
//indent this part
a = b;
}

NOT

if()
{
a = b;
}

i prefer the first way better```

Page 1 of 2 12 Last