8
\$\begingroup\$

I have made a program to weigh bags of money, and calculate whether too many/little coins are in the bags, and the difference.

bagWeight = 0
totalCoinsAdd = 0
totalCoinsRemoved = 0
totalBagsChecked = 0

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 = ""
    denom = ['0.01', '0.02', '0.05', '0.10', '0.20', '0.50', '1', '2']
    while denomination not in denom:
        denomination = input("What denomination is the money? £")

    idealWeight = {    '2' : 120,    '1' : 190,    '0.50' : 160,    '0.20' : 250,   '0.10' : 325,    '0.05' : 325,   '0.02' : 356,    '0.01' : 356,    }
    #This dictionary stores the ideal weight of each bag.
    bagCoins = {    '2' : 10,    '1' : 20,    '0.50' : 20,    '0.20' : 50,   '0.10' : 50,    '0.05' : 100,   '0.02' : 50,    '0.01' : 100,    }
    #This dictionary stores how many coins are in each bag.
    coinWeight = {    '2' : 12,   '1' :  9.5,   '0.50' : 8,  '0.20' : 5,   '0.10' :  6.5,  '0.05' : 3.25,'0.02' : 7.12, '0.01' : 3.56, }
    #This dictionary stores how much the 1 of each coin weighs. The values are in grams.
    AddCoins = True     #This defines a variable for whether to add/remove coins.    
    differentWeight = idealWeight[denomination] - bagWeight
    #This calculation works out how the difference in weight between the 'ideal' bag weight and the actual bag weight.
    coinInBag = bagWeight/coinWeight[denomination]
    #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 = bagCoins[denomination] - coinInBag
    #Calculating the amount of coin difference in the bag

    if differentWeight < 0:
        AddCoins = False
        differentWeight = differentWeight  * -1
        coinChange = coinChange * -1

    if AddCoins == True:
        print('The bag is ',str(differentWeight), 'g less than it should be')
        print('Add ', int(coinChange), ' coin(s)')
        totalCoinsAdd = totalCoinsAdd + coinChange

    if AddCoins == False:
        print('The bag is ',str(differentWeight), 'g more than it should be')
        print('Remove ', int(coinChange), ' coin(s)')
        totalCoinsRemoved = totalCoinsRemoved + 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: ', int(totalCoinsAdd))
print('Total coins removed: ', int(totalCoinsRemoved))
print('Total bags checked:', int(totalBagsChecked))
\$\endgroup\$
10
\$\begingroup\$

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.

\$\endgroup\$
  • 2
    \$\begingroup\$ +1 for that formatting article. Also, really nice answer :) \$\endgroup\$ – Grajdeanu Alex. Dec 6 '16 at 10:16
4
\$\begingroup\$

I would say that you should combine the lists into one. eg

coins = {'1p':[356,3.56,0.01]}

this could be done for all of the coins. the last value ids the monetary value of the coin in pounds.

This is my example:

#A variable to count the number of bags processsed
count = 0
#The function to check the weights of the bags and display the number of coins needed to be added or removed
def check(bag_weight, coin_type, bag_weight1):
    #import the variable 'count' into the function
    global count
    #Add 1 to the number of bags counted
    count = count+1
    #Find the maximum number of coins allowed in the bag
    x = bag_weight1[coin_type]
    y = x[1]
    #Check if the bag exceeds the maximum weight allowed
    if bag_weight > y:
        #If it does take the maximum away from the actual weight and display how much too heavy the bag is
        print("you have",int(bag_weight - y), "too many coins")
    else:
        #Otherwise take the actual weight from the maximum weight and display number of coins needed to be added
        print("you need to add",int(y - bag_weight), "coins")





#Starting function to get all necassary information for calculation to begin
def main():
    #Forever iterate through this loop
    while True:
        #Display the number of bags already checked
        print("you have checked", count, "bags.")
        #Ask if the user wants to weigh a bag
        y = input('do you want to check a bag?(Y/N)')
        #If answer is yes continue, other wise jump to the 'else' statement
        if y.lower() == 'y':
            #Get user to input the weight of the bag
            weight = float(input('how heavy is the bag(grams)?'))
            #Dictionary of lists containing vital information about the different types of coin the format: weight of coin, maximum number of coins allowed in the bag
            coin_Weight = {'1p':[3.56, 100],'2p':[7.12, 50],'5p':[3.25,100],'10p':[6.5, 50],'20p':[5.0, 50],'50p':[8.00, 20],'£1':[9.50, 20],'£2':[12.00, 10]}
            #Input the type of coin in the bag
            coin_type = input('what type of coin is it?(1p, 2p, 5p, 10p, 20p, 50p, £1, £2)')
            #Get the corresponding list of information from the dictionary about the coin
            bag_weight1 = coin_Weight[coin_type]
            #Divide the actual weight of the bag by the weight of one coin to find the number of coins in each bag
            bag_weight = weight/bag_weight1[0]
            #Output the number of coins in the bag allong with the type of coin
            print('there are', int(bag_weight), coin_type, 'coins in the bag.')
            #Initiate further calculation by passing the following arguments into the function 'check'
            check(bag_weight, coin_type, coin_Weight)
        #If the user answered 'no' continue into this part of the loop
        else:
            #Print a message thanking the user for using the program
            print("Thank you for using this program.")
            #Break out of the loop
            break
#Call the function 'main' to begin the program
main()
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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