23
\$\begingroup\$

This is the fifth project in my CS1 class. It's a bit more drab than my past projects, so my titles are getting worse unfortunately. :(

Write a program that computes the annual after-tax cost of a new house for the first year of ownership. The cost is computed as the annual mortgage cost minus the tax savings. The input should be the price of the house and the down payment. The annual mortgage cost can be estimated as \$ 3\% \$ of the initial loan balance credited toward paying of the loan principal plus \$ 6\% \$ of the initial loan balance in interest. The initial loan balance is the price minus the down payment. Assume a \$ 35\% \$ marginal tax rate and assume that interest payments are tax deductible. So, the tax savings is \$ 35\% \$ of the interest payment. Your program should use at least two function definitions and should allow the user to repeat this calculation as often as the user wishes.

I may have cheated a bit with my required two function definitions...

mortgage.cpp:

/**
 * @file mortgage.cpp
 * @brief Computes the annual after-tax cost of a new house
 * @author syb0rg
 * @date 10/9/14
 */

#include <cctype>
#include <iostream>
#include <limits>

/**
 * Makes sure data isn't malicious, and signals user to re-enter proper data if invalid
 */
long double getSanitizedDouble()
{
    long double input = 0.0L;
    while(!(std::cin >> input) || input < 0)
    {
        // clear the error flag that was set so that future I/O operations will work correctly
        std::cin.clear();
        // skips to the next newline
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        std::cout << "Invalid input.  Please enter a positive number: ";
    }
    return input;
}

/**
 * Safetly grabs and returns a lowercase version of the character (if the lowercase exists)
 */
char32_t getSanitizedChar()
{
    // absorb newline character (if existant) from previous input
    if(std::cin.peek() == '\n') std::cin.ignore();
    return std::tolower(std::cin.get());
}

int main()
{
    do
    {
        long double housePrice = 0.0L;
        long double downPayment = 0.0L;

        // get input for house price, re-read input if not a positive number
        std::cout << "Enter the price of the house: $";
        housePrice = getSanitizedDouble();

        // get input for down payment, re-read input if not a positive number
        std::cout << "Enter the down-payment: $";
        downPayment = getSanitizedDouble();

        long double loanBalance = housePrice - downPayment;
        long double interest = .06 * loanBalance;
        long double annualMortgage = (.03 * loanBalance) + interest;
        long double savings = .35 * interest;
        long double totalCost = annualMortgage - savings;

        std::cout << "The annual after-tax cost of the house is $" << totalCost << std::endl;

        std::cout << "Run the program again (y/N): ";  // signify n as default with capital letter
    } while ('y' == getSanitizedChar());
}
\$\endgroup\$
  • 23
    \$\begingroup\$ I understand that this is homework, but I must still caution all readers about using floating point numbers when dealing with money. It's a big no-no. Round off errors will very quickly accumulate and money will get lost. stackoverflow.com/questions/3730019/… \$\endgroup\$ – Emily L. Oct 7 '14 at 19:44
  • 1
    \$\begingroup\$ @EmilyL. Very Office Space-like \$\endgroup\$ – sjagr Oct 8 '14 at 20:40
  • 1
    \$\begingroup\$ @sjagr Of course, the guys in Office Space took Superman III as their inspiration :) \$\endgroup\$ – Flambino Oct 9 '14 at 0:14
21
\$\begingroup\$

Some minor changes I would recommend:

  • Replace the magic numbers with symbolic constants. It will be easier to modify your program when these rates change.

    constexpr long double ANNUAL_MORTGAGE_COST = 0.03; 
    constexpr long double LOAN_PRINCIPAL_PLUS = 0.06;
    constexpr long double MARGINAL_TAX_RATE = 0.35;
    
  • Instead of doing the calculations inside main() I would suggest putting it into a function.

    long double totalPayment(long double housePrice, long double downPayment){
    
        long double loanBalance = housePrice - downPayment;
        long double interest = LOAN_PRINCIPAL_PLUS * loanBalance;
        long double annualMortgage = (ANNUAL_MORTGAGE_COST * loanBalance) + interest;
        long double savings = MARGINAL_TAX_RATE * interest;
        long double totalCost = annualMortgage - savings;
    
        return totalCost;
    }
    
\$\endgroup\$
  • 3
    \$\begingroup\$ Perhaps, const references instead? Just for efficiency and safety. \$\endgroup\$ – user54356 Oct 7 '14 at 17:28
  • 5
    \$\begingroup\$ @Dave It is generally considered that built-in types are fast enough to be passed by value. \$\endgroup\$ – Morwenn Oct 7 '14 at 17:29
  • 1
    \$\begingroup\$ You switched ANNUAL_MORTGAGE_COST and LOAN_PRINCIPAL_PLUS in the interest and annualMortgage calculations. This might have been more obvious if you'd named them PRINCIPAL_REDUCTION_RATE and INTEREST_RATE instead. \$\endgroup\$ – Brythan Oct 7 '14 at 17:30
  • \$\begingroup\$ @Brythan Good catch. \$\endgroup\$ – user54356 Oct 7 '14 at 17:37
14
\$\begingroup\$

A few minor things to complete what has already been said. Almost none of them are significant, but good practice is good practice.

  • You don't need to use std::endl: it prints a newline character and flushes the buffer, which is unneeded here. You can simply use \n. It won't make a difference in your program, but it is still good practice.

    Note: as pointed out by @Emily, it flushes the buffer for buffered input only, which isn't the case of std::cout, unless manually redirected to a file. I'll keep this comment here anyway, not because I still believe that it is good practice, but because knowing about it might still be useful.

  • housePrice and downPayment could be declared later and directly initialized with getSanitizedDouble. There is no point in declaring them at the beginning of your do {} while () block.

  • getSanitizedChar returns char32_t. While this is fine, it may make more sense to return an int (return type of std::tolower) or a char since you read from std::cin which is an instance of std::basic_istream<char>.

  • You used the wrong literal in while(!(std::cin >> input) || input < 0), it should be 0.0L (haha, found one :p).

  • Also, some say it is a matter of style, many don't like it, but it is good practice to put braces around single-statement ifs. Had every Apple developer done this, the infamous goto fail; bug wouldn't have been a thing.

\$\endgroup\$
  • 5
    \$\begingroup\$ Using \n over std::endl is a matter of style and I would prefer using the latter. Under some circumstances using \n can avoid a costly buffer flush but I consider consistently using \n a premature optimisation. More information: stackoverflow.com/a/25569849/2498188 \$\endgroup\$ – Emily L. Oct 7 '14 at 19:31
  • \$\begingroup\$ +1 for "some say it is a matter of style" - I've been coding if(condition)doif();else doelse(); for years, and only recently (on own accord) switched to if(condition){doif();}else {doelse();} - and I'm happy with it, mainly because I don't type those braces - I use IDE autoformatting/autocompletion, getting the pros of both solutions (safety+less typing) - although I can say that it's kind of irritating when I get leftover braces after furious edits, screwing the formatting for whole code... \$\endgroup\$ – vaxquis Oct 8 '14 at 13:47
10
\$\begingroup\$

You've cheated indeed. It is more in a spirit of the question to make a function calculating total cost:

double total_cost(double balance);

At least you'd be able to reuse it for total costs of years to come.

\$\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.