Multiple source of information
You have 4 data structures (1 list and 3 dictionnaries) whose content needs to be kept in sync because values in the list need to be keys of the dictionnaries and keys from the dictionnaries not in the list are pointless.
Maybe thing would be clearer if you had only a single source of truth. For instance, you could have a single dictionnary mapping values to idealWeight, bagCoins and coinWeight (whatever it means). Python offer simple ways to do so via tuples or namedtuples.
FOr instance, you could do something like:
from collections import namedtuple
bagWeight = 0
totalCoinsAdd = 0
totalCoinsRemoved = 0
totalBagsChecked = 0
CoinInfo = namedtuple('CoinInfo', ['idealWeight', 'bagCoins', 'coinWeight'])
COINS = {
'2': CoinInfo(120, 10, 12),
'1': CoinInfo(190, 20, 9.5),
'0.50': CoinInfo(160, 20, 8),
'0.20': CoinInfo(250, 50, 5),
'0.10': CoinInfo(325, 50, 6.5),
'0.05': CoinInfo(325, 100, 3.25),
'0.02': CoinInfo(356, 50, 7.12),
'0.01': CoinInfo(356, 100, 3.36),
}
def inputs():
global bagWeight
global totalCoinsAdd
global totalCoinsRemoved
#The above grabs the global variables for use in the function
bagWeight = int(input("How much does the bag weigh? "))
denomination = ""
while denomination not in COINS:
denomination = input("What denomination is the money? £")
coin = COINS[denomination]
AddCoins = True #This defines a variable for whether to add/remove coins.
differentWeight = coin.idealWeight - bagWeight
#This calculation works out how the difference in weight between the 'ideal' bag weight and the actual bag weight.
coinInBag = bagWeight/coin.coinWeight
#This calculation works out how many coins are in each bag. It does this by, dividing the bag weight by how much each coin weighs.
coinChange = coin.bagCoins - coinInBag
#Calculating the amount of coin difference in the bag
The AddCoinss
variable
The comment This defines a variable for whether to add/remove coins.
is very verbose and could be a simple Add/remove coins
.
By the way, there is a style guide for Python code called PEP 8 which is definitly worth reading and worth following if you don't have any good reason not to do it (legacy project, company code style, etc). Among other things, it says that snake_case
should be used for variable (instead of CamelCase
).
Also, the way we check it later on can be improved for 2 reasons :
when a variable (or any expression) is known to be a boolean, it is better to write if var
(resp. if not var
) instead of if var == True
(resp. if var == False)
. In a real life situation, you'd say "if it rains" rather than "if it is True that it is raining".
you check if AddCoins == True
and then just after if AddCoins == False
without changing the value of AddCoins
. This is exactly what the else
keyword is for. It makes the code much clearer to the reader because one knows at first glance that we'll go though one and only one of the block.
So far, we have:
add_coins = True # Whether to add/remove coins.
if differentWeight < 0:
add_coins = False
differentWeight = differentWeight * -1
coinChange = coinChange * -1
if add_coins :
print('The bag is ',str(differentWeight), 'g less than it should be')
print('Add ', int(coinChange), ' coin(s)')
totalCoinsAdd = totalCoinsAdd + coinChange
else:
print('The bag is ',str(differentWeight), 'g more than it should be')
print('Remove ', int(coinChange), ' coin(s)')
totalCoinsRemoved = totalCoinsRemoved + coinChange
Now, this variable is not so useful at all because we are performing the test that really matters (differentWeight < 0
) just before and we could handle the different processing directly here.
Also, here again, variable name can be improved and you could use var *= -1
instead of var = var * -1
.
The code would look like:
if different_weight < 0:
different_weight *= -1
coinChange *= -1
print('The bag is ',str(different_weight), 'g more than it should be')
print('Remove ', int(coinChange), ' coin(s)')
totalCoinsRemoved = totalCoinsRemoved + coinChange
else:
print('The bag is ',str(different_weight), 'g less than it should be')
print('Add ', int(coinChange), ' coin(s)')
totalCoinsAdd = totalCoinsAdd + coinChange
As pointed out in Mathias Ettinger's comment, you might as well get rid of the multiplication altogether and just consider the negative value when printing :
if different_weight < 0:
print('The bag is ',str(-different_weight), 'g more than it should be')
print('Remove ', int(-coinChange), ' coin(s)')
totalCoinsRemoved = totalCoinsRemoved - coinChange
else:
print('The bag is ',str(different_weight), 'g less than it should be')
print('Add ', int(coinChange), ' coin(s)')
totalCoinsAdd = totalCoinsAdd + coinChange
String formatting
Python offers different nice ways to format string and providing multiple arguments to print
is not the favorite way to do this. You can find documentation online about string formatting but pyformat.info provides a very nice summary.
Using this, you could write something like:
if different_weight < 0:
print('The bag is {}g more than it should be'.format(-different_weight))
print('Remove {} coin(s)'.format(-coinChange))
totalCoinsRemoved = totalCoinsRemoved - coinChange
else:
print('The bag is {}g less than it should be'.format(different_weight))
print('Add {} coin(s)'.format(coinChange))
totalCoinsAdd = totalCoinsAdd + coinChange
again = 'yes'
while again != 'no':
inputs()
totalBagsChecked = totalBagsChecked + 1
again = input('Would you like to weight another bag? ("no" to quit)')
print('Total coins added: {}'.format(totalCoinsAdd))
print('Total coins removed: {}'.format(totalCoinsRemoved))
print('Total bags checked: {}'.format(totalBagsChecked))
Global variables
In general, it is a bad idea to use global variables and it tends to show that there is a problem in the way your code is organised.
In any case, bagWeight
does not need to be a global variable.