Should I use loops or stick with nested if statements?

Question

I am new at programming. I am using python. I am making a program that will compare different interest rates for saving accounts in my country (Paraguay). Since some of the calculations are similar in the banks, but some differ, I'd like to know how to better structure the code so that it won't repeat.

Should I just keep using if statements? Is there a way to do it with loops? How should I think about it? The code I add below is just for one bank. If I wanted to add another one, how should I proceed? My natural inclination would just keep doing if statements :)

PD: If There is also any feedback on my code will be appreciated. Or some resources that you think would help me at this stage. Please be kind, I am still very insecure about all this.

def ahorro_cda_nominal_anual (ahorro_inicial, tiempo_en_dias, moneda):

  if moneda == 'dolares':
    if ahorro_inicial>= 5000: 

        if 0 < tiempo_en_dias < 90:
            dMin = 0.05
            dMax = 0.05

        if 90 <= tiempo_en_dias < 180: 
            dMin = 0.15
            dMAx = 0.75

        if 180 <= tiempo_en_dias < 365:
            dMin = 0.25
            dMax = 1.25

        if 365 <= tiempo_en_dias:
            dMin = 1.25
            dMax = 2.00
        monto_final_min = ahorro_inicial * tiempo_en_dias*dMin/365 + ahorro_inicial
        monto_final_max = ahorro_inicial * tiempo_en_dias *dMax/365 + ahorro_inicial
        print ("Obtendrias minimamente " + str(round(monto_final_min/1000,3)) + " mil dolares.")
        print ("Hasta un valor maximo de " + str(round(monto_final_max/1000,3)) + " mil dolares.")
        return (monto_final_min, monto_final_max)

    else:
        print ("El valor minimo es de 5000 dolares. Necesitas " + str(5000 - ahorro_inicial) + " dolares mas.")

  elif moneda == 'guaranies':
    if ahorro_inicial >= 5000000:

        if 0 < tiempo_en_dias < 90:
            gMin = 0.25
            gMax = 2.5

        if 90 <= tiempo_en_dias < 180: 
            gMin = 0.75
            gMax = 2.5

        if 180 <= tiempo_en_dias < 365:
            gMin = 1.0
            gMax = 4.5

        if 365 <= tiempo_en_dias:
            gMin = 1.5
            gMax = 5.5

        monto_final_min = ahorro_inicial * tiempo_en_dias*gMin/365 + ahorro_inicial
        monto_final_max = ahorro_inicial * tiempo_en_dias *gMax/365 + ahorro_inicial
        print ("Obtendras minimamente " + str(round(monto_final_min/1000000,1)) + " milllones de guaranies.")
        print ("Y a lo sumo " + str(round(monto_final_max/1000000,1)) + " millones de guaranies.")
        return (monto_final_min, monto_final_max)

    else:
        print ("El monto minimo es 5 millones de guaranies. Necesitas " + str(5000000 - ahorro_inicial) + " guaranies mas.")
  else:
    print ("Solo aceptamos guaranies o dolares.")

ahorro_cda_nominal_anual (50000000, 180,'guaranies')

Answer 1

There are multiple ways to go about it... If your calculations differ only by the data(intrest rate, years, etc) then i would propose one of the above answers following the data driven approach.

If your calculations itself differ significantly (may be different banks have different clause) then rather than having one method with complex if-logic it is clear to separate it out in to different methods and call them with a jump table.

Simple example:

#!/usr/bin/env python

def bank_type_1(val1, val2):
    print 'bank type 1 calculations...'


def bank_type_2(val1, val2):
    print 'bank type 2 calculations...'



bank_methods = {
    'bank1': bank_type_1,
    'bank2': bank_type_2,
    'bank1_1': bank_type_1
}

def calculate(bank, val1, val2):
    if bank in bank_methods:
        bank_methods[bank](val1, val2)


if __name__ == '__main__':
    calculate('bank1', 10, 100)
    calculate('bank2', 10, 100)
    calculate('bank1_1', 10, 100)

Answer 2

Assuming only the rates and initial amounts change between banks, I suggest using a data structure that holds all config data (for each bank and each currency). Maybe like this, where you can add as many banks and currencies as needed:

bank_dict = {
    'bank_1': {
        'USD': {
            'min_amount': 5000,
            'rounding': 3,
            'rates': [
                (0, 0.05, 0.05),
                (90, 0.15, 0.75),
                (180, 0.25, 1.25),
                (365, 1.25, 2.00),
            ],
        },
        'GS': {
            'min_amount': 5000000,
            'rounding': 1,
            'rates': [
                (0, 0.25, 2.50),
                (90, 0.75, 2.50),
                (180, 1.00, 4.50),
                (365, 1.50, 5.50),
            ],
        },
    },
    'bank_2': {
        'EUR': {
            'min_amount': 8000,
            'rounding': 3,
            'rates': [
                (0, 0.05, 0.05),
                (90, 0.15, 0.75),
                (180, 0.25, 1.25),
                (365, 1.25, 2.00),
            ],
        },
    },
}

I suggest using the python convention about raising errors in case of anomalous data, like an non-existing bank or currency. The code could look like this:

def calculate(bank_id, currency_id, initial_amount, days):
    try:
        bank_data = bank_dict[bank_id]
    except KeyError:
        raise ValueError('bank "{}" does not exist'.format(
            bank_id))

    try:
        currency_data = bank_data[currency_id]
    except KeyError:
        raise ValueError('currency "{}" does not exist for bank "{}"'.format(
            currency_id, bank_id))

    if initial_amount < currency_data['min_amount']:
        raise ValueError('initial amount is {} but should be at least {}'.format(
            initial_amount, currency_data['min_amount']))

    # search for rates
    selected_rate_min = 0
    selected_rate_max = 0
    for rate_days, rate_min, rate_max in currency_data['rates']:
        if days < rate_days:
            # exit loop, because the previous rates should stay as selected rates
            break

        selected_rate_min = rate_min
        selected_rate_max = rate_max

    interest_min = round(
        initial_amount * days * selected_rate_min / 365,
        currency_data['rounding'])
    interest_max = round(
        initial_amount * days * selected_rate_max / 365,
        currency_data['rounding'])

    final_amount_min = initial_amount + interest_min
    final_amount_max = initial_amount + interest_max

    print(
        bank_id, currency_id, initial_amount, days,
        selected_rate_min, final_amount_min,
        selected_rate_max, final_amount_max)

    return final_amount_min, final_amount_max

Note that the code will not grow when you add more banks or currencies (assuming that all banks use the same core logic implemented here).

---

A final note: if you need exact rounding for money values, you should look into Pythons decimal data type instead of using float (for example 0.15 is a float).

Answer 3

There are definitely a few ways you can improve your code to avoid repeating yourself as much as you currently do. Avoiding repetition is good because it means you are less likely to have some subtle bug in one corner case that you didn't detect because you only tested other cases that work correctly.

To start with, I'd suggest making a data structure that contains all the data you need to determine the rates for a given loan request. In your current code, that looks to be the currency and minimum loan amount, and then the different interest rates for different loan durations. I suggest a dictionary keyed by currency, who's values are 2-tuples, min_loan, rates. The rates value is a list of duration, min_rate, max_rate three-tuples.

data = {
    'dolares': (5000, [
        (90, 0.05, 0.05),
        (180, 0.15, 0.75),
        (365, 0.25, 1.25),
        (math.inf, 1.25, 2.00),
    ]),
    # similar rates for 'guaranies' would go here, omitted for brevity
}

def ahorro_cda_nominal_anual(ahorro_inicial, tiempo_en_dias, moneda):
    if moneda not in data:
        raise ValueError("Unknown currency {!r}".format(moneda)) # or whatever error handling you want

    min_loan, rates = data[moneda]
    if ahorro_inicial < min_loan:
        raise ValueError("Loan is too small, minimum is {} {}.".format(min_loan, moneda))

    for duration, dMin, dMax in rates:
        if tiempo_en_dias < duration:
            break # stop looping, keeping dMin and dMax at the current values

    monto_final_min = ahorro_inicial * tiempo_en_dias*dMin/365 + ahorro_inicial
    monto_final_max = ahorro_inicial * tiempo_en_dias*dMax/365 + ahorro_inicial
    return (monto_final_min, monto_final_max)

I left out the printed output of your current function, mostly because the formatting was different between the different currencies you were using in the original version (and I don't speak Spanish well enough to fix it up). You can pretty easily put it back in, as long as you can come up with some formatting that works consistently for every currency. If a generic message isn't good enough, you could add a format string or two to the data structure, so the generic code can use different language wherever you need it.