I am trying to create an EMI calculator for a loan. The problem description is:

Given a principal amount, rate of interest, monthly or quarterly installment terms and duration of loan in months/years and print EMI amount. Input format:Principal, Rate of Interest per year, M/Q for Monthly or quarterly installment, duration of year in Year (Y) or Month (M) Example Inputs:

10000, 12, M, 2Y: Principal = 10000, Rate of Interest = 12%, Monthly installment, Duration = 2 years

50000, 10.5, Q, 36M: Principal = 50000, Rate of Interest = 10.5%, Quarterly installment, Duration = 36 months

What are the flaws in this code? How can I perfect it?

def EMI_calculator(P,R , M,d):
#set rate of interest monthly
    print type(R)
    r= (R/(12.0 *100))
    print r
#chechk if monthly or quaterly installment
    if M== 'Q': 

    if d[-1] =='Y':
        print "duration in years"
        n = d[0:(len(d)-1)]
        print n
        m = 12 *int(n)
        print m
    elif d[-1] =='M':
        print "duration is in months"
        n = d[0:(len(d)-1)]
        print n   
        m = 1 *int(n) 
        print m
    EMI = x*(P*r*(1+r)**m)//((1+r)**m - 1)
    return EMI

print EMI_calculator(10000,12,'Q','3Y')
  • 1
    \$\begingroup\$ Can you specify what EMI is and how it should be calculated, besides in the code? \$\endgroup\$ Jan 3, 2018 at 22:09

1 Answer 1


In the comments to your other questions some things were mentioned that are also applicable to this question as well, like:

  1. Try to follow PEP 8 style guide.

  2. Single letter names should be avoided.

  3. Avoid useless comments, make your code self explanatory.

  4. Check for invalid input, raise exceptions.

Now, you have some magic numbers like 12, 100 and 4. Give them names. Make them constants. Don't leave them like that.

Some prints don't make a lot of sense, like printing type of R or printing if duration is in years or in months. That looks more like information for debugging. I suggest you to take a look at logging module and give it a try. But for such a small program it is probably an overkill.

This n = d[0:(len(d)-1)] is repeated twice in your code and it can be taken out of that if-elif statement. Moreover it can be simplified as n = d[:-1].

Instead of writing

EMI = x*(P*r*(1+r)**m)//((1+r)**m - 1)
return EMI

you can write just

return x*(P*r*(1+r)**m)//((1+r)**m - 1)

Finally, wrap your print EMI_calculator(10000,12,'Q','3Y') like this:

if __name__ == '__main__':
    print EMI_calculator(10000, 12, 'Q', '3Y')

You can read about it here.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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