2
\$\begingroup\$

I needed to create a mortgage calculator for an intro to CS class. As part of the assignment, with an interest rate of 6% needs to change to 7% after 3 years hence that if statement. The char dummy line at the end is a requirement from my professor.

I'm mainly looking for ways to clean it up or if I missed anything. I'm required to use namespace std, and I know a lot of you don't care for it.

#include <iostream>
using namespace std;

int main()
{
    double monthlyPayment;
    double balance;
    double interestRate;
    double interestPaid;
    double initialBalance;
    double termOfLoan;
    double month = 1;

    cout.setf(ios::fixed);  
    cout.setf(ios::showpoint);
    cout.precision(2);

    cout << "Enter the current balance of your loan: $";
    cin >> balance;
    cout << "Enter the initial yearly interest rate : ";
    cin >> interestRate;
    cout << "Enter the desired monthly payment : $";
    cin >> monthlyPayment;

    initialBalance = balance;

    while (interestRate >= 1)       /*Converts the interest rate to a decimal if the user inputs 
                                                      in percentage form*/
    { 
        interestRate = interestRate / 100; 
    }

    if(month >= 36);
    { 
        if(interestRate=.06)
        {
            interestRate=.07;
        }
    }
    balance = balance * (1 + interestRate / 12) - monthlyPayment;

    cout << "After month 1 your balance is $" << balance << endl;

    while (balance > 0)
    {
        if (balance < monthlyPayment)
        {
            balance = balance - balance;
        }   
        else 
        {
            balance = balance * (1 + interestRate / 12) - monthlyPayment;
        }

        month = month++;
        month = month + 1;
        month += 1;     

        cout << "After month " << month << ", your balance is : $" << balance << endl;
        }
        cout << "You have paid off the loan at this point. Congratulations!" << endl;


        termOfLoan = month;

interestPaid = (monthlyPayment * termOfLoan) - initialBalance;      

/*I believe the formula above would work if only there was
 a way to calculate how many months it took to pay 
off the loan, but since it varies, I don't know how to 
calculate "termOfLoan". */

            cout << "You paid a total ammount of $" << interestPaid << " in intrest." << endl;
            cout << "Total number of months = " << month << "." << endl;

char dummy;
cout << "Enter any key to quit." << endl;
cin >> dummy;
}
\$\endgroup\$
1
  • \$\begingroup\$ When you say the interest rate changes from 6% to 7% after 3 years, does this mean that it increases by 1% every 3 years? \$\endgroup\$
    – T145
    Feb 26, 2016 at 23:37

3 Answers 3

2
\$\begingroup\$
  1. You use double values for calculations. Especially for money, I would suggest to switch to int and handle it in cents. That way you can avoid problems with floating point precision.
  2. You don't check if cin>> fails or succeeds. If the user doesn't enter a number you may have a problem here. A possible solution might be to let the user reenter the value as long as it isn't correct.

    do
    {
        cout << "Enter the current balance of your loan: $";
    }
    while (!(cin >> balance));
    
  3. You have a wrong check in the line if (interestRate = .06). You probably meant if (interestRate == .06). Additionally, this check might also be a problem as you do some calculations on interestRate that may cause inprecision (again floating point precision problem).

    As you obviously await the user to input the value as int your check should be against 6 before you divide the value with 100.

  4. You have this piece of code:

    if (balance < monthlyPayment)
    {
        balance = balance - balance;
    }
    

    Which is the same as:

    if (balance < monthlyPayment)
    {
        balance = 0;
    }
    

    I think the second version is easier to understand.

I didn't check any more of the code yet. I especially didn't prove it for correctness in you calculations.

\$\endgroup\$
3
  • \$\begingroup\$ That's very helpful! What's the advantages of using int here over double? My very basic understanding of it is that double is better suited to handle decimals. \$\endgroup\$ Feb 26, 2016 at 17:16
  • 1
    \$\begingroup\$ @JonathanHall I would recommend you to have a look at these examples. There is also an article about the problems with floating point precision. But see for yourself: coliru.stacked-crooked.com/a/96637ffdb47780d4 \$\endgroup\$ Feb 26, 2016 at 17:36
  • \$\begingroup\$ @JonathanHall: Please be aware that if you use integer values these are always round to the lower bound. 3/2 is 1 and not 1.5. \$\endgroup\$ Feb 27, 2016 at 1:36
2
\$\begingroup\$

Use of while where if is appropriate

You have:

while (interestRate >= 1)
{ 
   interestRate = interestRate / 100; 
}

I believe that should be an if.

if (interestRate >= 1)
{ 
   interestRate = interestRate / 100; 
}

Wrong place for if (month >= 36)

You have:

if(month >= 36);
{ 
   if(interestRate=.06)
   {
      interestRate=.07;
   }
} 

The first error is that you have an errant ; in the if line.

if(month >= 36);  // This terminates the if statement

The block

   if(interestRate=.06)
   {
      interestRate=.07;
   }

is executed no matter what.

Also, as has already been pointed out by the other answer, you need to use

   if(interestRate == .06)

The other point is that you need to move this check inside the subsequent while loop. Otherwise, the conditional month >= 36 will always evaluate to false since month has been initialized to 1.

Updating month

Inside the while loop, you have:

  month = month++;     // This is bad code. It leads to UB.
  month = month + 1;   // Increment month by 1
  month += 1;          // Increment month by 1 again.

It's not clear to me what you are trying to do here.

I think you should change the type of month from type double to int and have just one line to increment its value.

This is how I see the while loop.

int month = 1;

...


while (balance > 0)
{
   if(month >= 36)
   { 
      if ( interestRate == .06 )
      {
         interestRate = .07;
      }
   }

   if (balance < monthlyPayment)
   {
      balance = 0;
   }   
   else 
   {
      balance = balance * (1 + interestRate / 12) - monthlyPayment;
   }

   cout << "After month " << month << ", your balance is : $" << balance << endl;

   ++month;
}
\$\endgroup\$
2
\$\begingroup\$

Bug

if(interestRate=.06)

You should use ==, as explained in StackOverflow

Right now it counts as always true, look at this example:

~$ cat ifeq.c
int main() {
  int x = 1;
  if (x = 2) {
    puts("hi");
  }
}
~$ gcc ifeq.c && ./a.out
hi

Listen to the compiler

This could have been avoided easily if you compiled with -Wall that I highly recommend:

~$ gcc -Wall ifeq.c && ./a.out
ifeq.c:3:3: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
   if (x = 2) {
   ^
// other warnings
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.