0
\$\begingroup\$

I wanted to create a loan calculator, but the code I could think of was using 3 functions, and calling them and getting result. But I felt it was easy but bad code because I thought this can be written in fewer lines and with more quality. I wanted to ask what are some things to keep in my mind while trying to write better code instead of going in flow and creating many functions.

#include<stdio.h>
#include<stdlib.h>
#include<math.h>

float loanAmount,interestRate,Discount,i,loanPayment; //global variables

int years;

void loanDetails(void); //input from user regarding the loan
float annualRate(void);// interest rate 
float discount(void); //discount to be added

int main() //main
{

    loanDetails(); //called three functions
    annualRate();
    discount();

    loanPayment = (loanAmount)/(Discount); //formula for monthly payments

    printf("\nthe monthly loan payment is %f",loanPayment);

    return EXIT_SUCCESS;
}

void loanDetails(void) //taking input from user
{
    printf("please enter Total Loan Amount: ");
    scanf("%f",&loanAmount);
    printf("please enter the number of years: ");
    scanf("%d",&years);
    printf("please enter the annualrate to be applied: ");
    scanf("%f",&interestRate);
}

float annualRate(void) //calculated annual rate
{
    float *ann;
    ann = &interestRate;
    i = ((*ann)/(100))/12;
    printf("the value for %d years is %f",years,i);
    return i;
}

float discount(void) //calculated dicount
{    
    float *x,y;
    x = &i;
    y = 1 + *x;
    int n = years*12;

    float topD = (pow((y),n)-1);
    float botD = (y-1)*pow((y),n);
    Discount = topD/botD;
    printf("\nthe value of discount is : %f",Discount);
    return Discount;
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! While I think the main charm of the traditional C indent of 8 positions is discouraging heavily nested structures, please indent your code consistently. \$\endgroup\$ – greybeard Feb 23 at 6:28
  • 1
    \$\begingroup\$ Some things are unclear. For example, when calculating the annual interest rate, you do i = ((*ann)/(100))/12;, which implies you're calculating a monthly rate because you're dividing by 12. You're also relying too much on global variables instead of utilizing function parameters. \$\endgroup\$ – AleksandrH Feb 24 at 1:46
1
\$\begingroup\$

There are a few things to note here. There is extensive referencing and dereferencing of variable addresses, which makes the code less clear. For example, float *x,y; x = &i; y = 1 + *x;. This is unnecessary and seems imposed by thinking related to your use of global variables.

There is nothing wrong with passing pointers as arguments to functions, rather than using global variables.

There is also nothing wrong with using functions. The code below is clearer. Additionally, it removes the need for the function "annualRate," as that is simply a division.

#include<stdio.h>
#include<stdlib.h> 
#include<math.h>

void loanDetails(float * loanAmount, int * years, float* interestRate);
float discount(int years, float i);



int main (void) {
    float loanAmount, interestRate, disc;
    int years;

    loanDetails(&loanAmount, &years, &interestRate);
    disc = discount(years, interestRate/1200);

    printf("\nthe value of discount is : %f",disc);


    printf("\nthe monthly loan payment is %f",loanAmount/disc);
    return EXIT_SUCCESS;
}

void loanDetails(float * loanAmount, int * years, float* interestRate) {
    printf("please enter Total Loan Amount: ");
    scanf("%f",loanAmount);
    printf("please enter the number of years: ");
    scanf("%d",years);
    printf("please enter the annualrate to be applied: ");
    scanf("%f",interestRate);
}
float discount(int years, float i) {
    int months = years*12;
    return (pow((1+i),months)-1)/((i)*pow((1+i),months));
}

If you wanted no functions other than main, you could do the following. Essentially, move the code from the functions to main. This is appropriste, considering that the code is not repeated, runs sequentially, and does not have varied/multiple inputs.

#include<stdio.h>
#include<stdlib.h> 
#include<math.h>


int main (void) {
    float loanAmount, interestRate, disc, i;
    int years, months;

    printf("please enter Total Loan Amount: ");
    scanf("%f",&loanAmount);
    printf("please enter the number of years: ");
    scanf("%d",&years);
    printf("please enter the annualrate to be applied: ");
    scanf("%f",&interestRate);

    months = years*12;
    i = interestRate/1200;

    disc = (pow((1+i),months)-1)/((i)*pow((1+i),months));

    printf("\nthe value of discount is : %f",disc);

    printf("\nthe monthly loan payment is %f",loanAmount/disc);
    return EXIT_SUCCESS;
}
\$\endgroup\$
1
\$\begingroup\$

Prefer the C++ standard library headers <cmath>, <cstdlib> and <cstdio> (though prefer <iostream> to the latter; it will help avoid some common errors). These place the C names into the std namespace where they belong.

In C++, we normally write () to indicate that a function takes no arguments. (void) is a C style of code.

Prefer double to float for floating-point calculations. double is the "natural" type in C and C++; use float only where space is the main concern.

Avoid global variables. In larger programs, global variables make it much harder to reason about parts of the code in isolation, so don't get into bad habits like this in your small programs.

I/O operations can fail. There's no excuse for unchecked scanf(), which can lead to undefined behaviour.

There's no need to take the address of interestRate here, nor to divide separately by 100 and then by 12:

float *ann;
ann = &interestRate;
i = ((*ann)/(100))/12;

We can just use it:

// convert annual percentage rate to monthly rate as a fraction
double i = interestRate / 1200;

Similarly,

float *x,y;
x = &i;
y = 1 + *x;

can be simply

double y = 1 + i;

When printing output, lines normally end with a newline, rather than beginning with one. Many terminals and/or stdio implementations flush output when they receive a newline (that's certainly true if stdout is a line printer, for example), and we certainly don't want to exit without a final newline (the next shell prompt then appears in an unexpected position).

There's far more parentheses than useful here:

float topD = (pow((y),n)-1);
float botD = (y-1)*pow((y),n);

It's probably worth pulling out the common subexpression std::pow(y,n), too (and months might be a better name than n).

Why is Discount named with a capital D?

Many of the comments aren't very helpful:

int main() //main
{

    loanDetails(); //called three function

We can see that it's the main() function, and we can see that three functions are called. What we can't see is that the functions interact invisibly (via the global variables), and so must be called exactly in that order. That's the sort of information that comments are useful for.

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