Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm making a few forms of a mortgage calculator for my intro to computer science class. With inputs, I currently have it outputting the monthly payments. I'll eventually be outputting how long it will take to pay off the loan once I add some code. I'm just looking for suggestions for any easier/quicker ways to do it. It currently compiles and runs fine.

#include "math.h"
#include <iostream>
using namespace std;

int main()
{
   int a; //amount of the loan
   double i; // the loan interest rate
   int y; //years of the loan
   int t = 12; //loan term in months
   double mPayment; //variable for ouputting the payment

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

   cout << "\n Enter the amount of loan in $ = ";
   cin >> a;
   cout << "\n Enter the interest rate = ";
   cin >> i;
   cout << "\n Enter years of the loan = ";
   cin >> y;

   mPayment = (a * i) / (1 - pow(1+i,-t)); //Formula to figure mortgage payment amount

   cout<< "\nYour Monthly Payment Amount is: $"<< mPayment; //prints out montyly payment amount

   return 0;
}
share|improve this question
    
As stated on SO before this was moved: it's a good idea to give your variables meaningful names. On a small project this won't be a problem. On a large project when you have hundreds or even thousands of lines of code will "a", "i", "t", and "y" mean anything to you if/when you have to fix a bug in this code? – Conduit Feb 24 at 19:06
    
Ya, I saw that response! I'll be sure to do that. Great suggestion. I also had no idea that this sort of question wasn't appropriate for overflow :O – Jonathan Hall Feb 24 at 19:07
    
No worries - the community can be overbearing about it. I tried to resolve exactly what happened there ages ago. – Conduit Feb 24 at 19:08
  • "math.h" should be <cmath>. The brackets are for system libraries whereas the quotes are for user-defined headers. The .h ending is mostly for C.

  • You don't actually need return 0 at the end of main(). It will do this same return for you at the very end.

  • You should consider input validation, at least if you decide to expand on this outside of the class. You could even allow the user the option to input via the command line, making this a bit more convenient.

  • You don't need those last two comments since they're already obvious. Only use them where explanation is needed for you and/or the reader.

  • Consider having the calculation done in a function for better maintainability:

    // add this to better explain that value
    const int loan_term = 12;
    
    double calculateMortgagePayment(int loan, double interest_rate)
    {
        return (loan * interest_rate) / (1 - pow(1 + interest_rate, -loan_term));
    }
    
    int main()
    {
        // ...
    
        // could also be const since value shouldn't change later
        const double mortgagePayment = calculateMortgagePayment(loan, interest_rate);
    
        // ...
    }
    

    I've taken into account@Conduit's advice regarding variable names. I've also kept the double return type, which isn't really ideal for money. There are resources online that explain more about that.

share|improve this answer

Two things I noticed:

It's a good idea to give your variables meaningful names. On a small project this won't be a problem. On a large project when you have hundreds or even thousands of lines of code will a, i, t, and y mean anything to you if/when you or someone else has to fix a bug in this?

In C++ using namespace std, as you have likely done above, can be a bit dangerous. It makes things nicer to look at when you can use cout instead of std::cout, but the std namespace is very, very large. Your chances of running into namespace pollution (bringing in classes/functions/etc. from the namespace that have the same name as things in your code) are surprisingly high. I advise you stick to adding on the std::... even though it's a bit ugly.

share|improve this answer
    
I changed the simply by typing out a full name for the variables. a=amount, i=interest rate, t=term, y=years now. It's outputting very odd though. For example, if the loan is $250,000 at 6% over 20 years, it's saying the monthly payment is over $29,000. I also didn't know about the using namespace std; Every beginner program example we've used in class uses it. I'll change it and see what my professor says. – Jonathan Hall Feb 24 at 19:20
1  
@JonathanHall There is a somewhat frustrating trend where people are taught with using namespace std because it's simpler, but then are never trained out of it. If you are advanced enough in your learning to understand what namespacing is, what it is used for, and why using namespace std is dangerous, then I would do away with it immediately. If any of those things are beyond your grasp at this point then keep it in, but remember to consider losing it later! – Conduit Feb 24 at 20:11
1  
@JonathanHall Also: looks like there might be a maths error, then - calculating by hand yields the same result. Look into the formula you're using to make sure you didn't miss anything... – Conduit Feb 24 at 20:16

Wrong results

Your formula is off, mainly because of two reasons:

  1. You don't even use y after you read it from the input. So essentially you are always calculating a 1 year loan because you use t=12. You should multiply t *= y; to get the actual loan length.
  2. Your interest rate i should be a monthly interest rate. If the input is a yearly rate, you should divide i /= 12.0; to get the monthly rate.
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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