How to eliminate redundancy using a function (def)?

Question

I'm building a currency converter and I've pretty much finished the program. However, I am trying to eliminate redundancy by implementing a function or a define block. I've tried a number of things but it does not seem to be working.

My current code is like this:

EUR = 0.83

EGY = 16.22

def currency_converter():
  money = total_value
  new_value = 0
  if currency == 1:
    new_value = money*EUR_CON
    print("Your total is " + "$" + str(money) + " US Dollars  which is " + "e£ " + str(new_value) + " European Euros.")
  elif currency == 2:
    new_value = money*EGY_CON
    print("Your total is " + "$" + str(money) + " US Dollars  which is " + "e£ " + str(new_value) + " Egyptian Pounds.")

I want to essentially make the clause under the if/elif block a function. I've tried doing this:

def conversion(EUR_CON,GDP_CON, BRL_CON, EGY_CON, YEN_CON):
  new_value = money*conversion()
  print("Your total is " + "$" + str(money) + " US Dollars  which is " + str(new_value)+ str(conversion)

if currency == 1:
    conversion(EURO_CON)

But it's not working. Can someone help?

The `conversion` function 1) is recursive and has no escape path, and 2) has many parameters which are not being provided by either caller. — S3DEV, Feb 10 '21 at 19:08
Assuming you always start from the same currency and only want it to be converted to single other, I would put a single argument for a function, based on users response for which he wants to use. Then use apropriate value based on that response. Here you are requiring multiple arguments for a function, but declared only one when calling it. — JackTheCrab, Feb 10 '21 at 19:08

Barmar · Answer 1 · 2021-02-10T19:29:28.223

1

Put all your conversion rates into a list or dictionary, so you don't need all those if statements.

You don't need so many parameters to the function. Just the currency you're converting to and the amount. The function can then look up all the information related to the currency parameter.

conversion_data = {
    'EUR': {'rate': 0.83, 'symbol': '€', 'name': 'European Euros'},
    'EGY': {'rate': 16.22, 'symbol': '£', 'name': 'Egyptian Pounds'},
    ...
}

def conversion(currency, dollars):
    new_value = dollars * conversion_data[currency]['rate']
    return f"Your total is ${dollars} US dollars which is {conversion_data[currency]['symbol']}{new_value} {conversion_data[currency]['name']}."

print(conversion('EUR', 5))

edited Feb 10 '21 at 19:29

answered Feb 10 '21 at 19:11

Barmar

741,623
53
500
612

when i call the function conversion() what do I put within the parentheses? I put EUR, 0.83 but it is giving me an error message. – h.avery Feb 10 '21 at 19:25
I showed an example – Barmar Feb 10 '21 at 19:28
The first argument must be a string in quotes. The second is the amount of dollars, – Barmar Feb 10 '21 at 19:29

ShadowRanger · Answer 2 · 2021-02-10T19:25:25.913

The correct way to do this is to make a mapping or enum that ties the type of conversion to the associated parameters (in this case, the multiplier and the string name of the target currency). For example, with enum:

from enum import Enum

class Currency(Enum):
    EUR = 0.83, "European Euros"
    EGY = 16.22, "Egyptian Pounds"

def currency_converter(target_currency):
    multiplier, name = target_currency.value  # Retrieve value of enum and unpack parts for use
    new_value = money * multiplier
    print(f"Your total is ${money} US Dollars which is {new_value} {name}")

which then allows you to use it with just:

currency_converter(Currency.EUR)  # Convert to euros

To be clear: Using a dict for a similar purpose is perfectly fine as well. Enums mostly just emphasize that there are a fixed, known set of possible conversions, where dicts don't have that idea baked in as thoroughly (adding and removing keys is always a possibility).

I'll note that in real code, functions generally shouldn't rely on receiving non-constant information from globals, nor should they print the results (returning them allows the caller to print, or not print, as they choose) so a better design would be something like:

def convert_usd_to_currency(target_currency, money):
    multiplier, _ = target_currency.value  # Retrieve value of enum and unpack parts for use
    return money * multiplier

possibly with a helper that does the printing (if you really have many places that need to format it the same way):

def print_converted_currency(currency, money):
    new_value = convert_usd_to_currency(currency, money)
    print(f"Your total is ${money} US Dollars which is {new_value} {currency.name}")

I'll admit to a failure of imagination here; I almost never see a need to factor out the output code itself (each location prints different things based on need), so I'd probably just inline that work in the one place that actually prints it (as opposed to potentially many places that need to perform the conversion).

How to eliminate redundancy using a function (def)?

2 Answers2