HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Currency Converter in Python 3

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pythonconvertercurrency

Problem

Beginner Python developer here.
I was tinkering around with Python and decided to build a currency converter. It takes in input from the user and prints out the converted currency values. The fixer.io API is used to get currency rates. Looking for any improvements that I can make in it.

GitHub

Code

```
# importing required libraries
import requests
import pycountry
from _datetime import datetime
from babel import numbers

def error_sev():
print("Sorry! There seems to be an error. Please check if your network is working")

def error_inp():
print("Sorry! There seems to be an error. Please check if the currencies entered are valid.")

def currency_print(input_cur, output_cur, input_currency_name, output_currency_name, amount, rate):
# printing out exchange rate
print("The rate for {} to {} as on {} is: "
.format(input_currency_name, output_currency_name, date.strftime("%d-%m-%Y")), end='')
print(numbers.format_currency(1, input_cur, locale='en') + " = " +
numbers.format_currency(rate, output_cur, locale='en'))

# printing converted value
print("\t", end='')
print(numbers.format_currency(amount, input_cur, locale='en') + " = " +
numbers.format_currency(amount * rate, output_cur, locale='en'))

print('-'*100)

# list of available currencies
currencies = [
'USD', 'JPY', 'BGN', 'CZK', 'DKK', 'GBP', 'HUF', 'PLN', 'RON', 'SEK', 'CHF', 'NOK', 'HRK', 'RUB', 'TRY',
'AUD', 'BRL', 'CAD', 'CNY', 'HKD', 'IDR', 'ILS', 'INR', 'KRW', 'MXN', 'MYR', 'NZD', 'PHP', 'SGD', 'THB',
'ZAR', 'ISK'
]

# printing the list of available currencies for the user
print("Available currencies: ", end='')
for item in sorted(currencies)[:-1]:
print(item, end=', ')
print(sorted(currencies)[-1])

# taking user input
try:
amount = float(input("Enter amount: "))
# checking for input errors
except ValueError:
print("Invalid input. Please enter only numbers.")

else:
# taking currency values as input from user

Solution

Code Style Improvements

-
"Flat is better than nested". You can make an early exit in case of invalid input:

import sys

try:
    amount = float(input("Enter amount: "))
except ValueError:
    print("Invalid input. Please enter only numbers.")
    sys.exit(1)


That will allow you to remove the else: part and continue on the top-level. Or, you can let the user retry the input until it is valid

  • on the same topic of decreasing nestedness depth - add more early exists. For instance, if input currencies are invalid, throw an error and exit. Then, remove the else: and continue with your "positive case" logic on the same level. This should improve overall readability



  • define the constants, like the list of currencies, as per PEP8 - in upper case (reference)



  • put the main execution logic to under if __name__ == '__main__':



  • you can simplify if output_cur != '': with just if output_cur:



  • I'm not sure why you are importing datetime from _datetime (with underscore). I would expect the import to be from datetime import datetime



  • don't put comment for obvious parts of the code. For example, "importing required libraries" does not provide any useful information.



-
organize imports per PEP8 - stdlib libraries, then a newline, third-parties, a new line and then your "local" dependencies, all sorted alphabetically:

from datetime import datetime

from babel import numbers
import pycountry
import requests


Other High-level ideas

  • define custom exceptions. Instead of using the error_sev and error_inp functions where you print errors, define custom exceptions like InvalidCountryValueError. Throw it with your custom message inside



  • since you are posting it on github, consider organizing the project properly - add requirements.txt with the list of dependencies, add more documentation, tests - see more at Open Sourcing a Python Project the Right Way



  • on the related topic: currently, there is only one way to use your program. Consider someone who wants to use your library as an API - not going through the standard in inputs, but calling a function asking for currency rates. Thinking about your program this way may help you to re-design it a bit, apply "Extract Method" and other refactoring methods. Also, if you would try to add tests, you will quickly realize that there is no easy way to unittest the program - usually a red flag when designing clean and modular APIs



Performance notes

-
I'd use a set to keep the supported list of currencies. Since you check the input currencies to be valid with in, this should have a positive impact on performance:

CURRENCIES = {
    'USD', 'JPY', 'BGN', 'CZK', 'DKK', 'GBP', 'HUF', 'PLN', 'RON', 'SEK', 'CHF', 'NOK', 'HRK', 'RUB', 'TRY',
    'AUD', 'BRL', 'CAD', 'CNY', 'HKD', 'IDR', 'ILS', 'INR', 'KRW', 'MXN', 'MYR', 'NZD', 'PHP', 'SGD', 'THB',
    'ZAR', 'ISK'
}

Code Snippets

import sys

try:
    amount = float(input("Enter amount: "))
except ValueError:
    print("Invalid input. Please enter only numbers.")
    sys.exit(1)
from datetime import datetime

from babel import numbers
import pycountry
import requests
CURRENCIES = {
    'USD', 'JPY', 'BGN', 'CZK', 'DKK', 'GBP', 'HUF', 'PLN', 'RON', 'SEK', 'CHF', 'NOK', 'HRK', 'RUB', 'TRY',
    'AUD', 'BRL', 'CAD', 'CNY', 'HKD', 'IDR', 'ILS', 'INR', 'KRW', 'MXN', 'MYR', 'NZD', 'PHP', 'SGD', 'THB',
    'ZAR', 'ISK'
}

Context

StackExchange Code Review Q#159466, answer score: 3

Revisions (0)

No revisions yet.