4
\$\begingroup\$

My first python program top to bottom used for work.

First it converts units from imperial to metric and vice versa, then adds the dimensions to a shipment and prints the shipment. When you do a new conversion/addition it adds it to the shipment so I can see what the final shipment is.

I have already done some pythonification of the code but I know it's far from optimal. Please share your advice in how to improve it.

Edit removed brackets around lists when checking 'in'.

import sys
import re
counter = 0
shpmnt = {}
volumes = {}
weights = {}

while True:
    #list of words to exit the program
    escape_word = ["exit", "q", "x", "quit", "escape", "no", "n"]
    #list of words to initiate volume calculation
    volume_calc = ["v", "vol", "volume"]

    #list of conversion factors        
    k_to_l = float(2.2046226218)
    l_to_k = float(0.45359237)
    c_to_i = float(0.393701)
    i_to_c = float(2.54)
    f3_to_m3 = float(0.0283168)
    i3_to_m3 = float(0.000016387064)
    c3_to_m3 = float(0.000001)
    i3_to_f3 = float(0.000578704)


    #initial input of conversion #
    num = input("\nEnter number to convert including unit"\
                "\nor v to calculate the last entries volume\n")
    #counter for each carton
    counter += 1

    #determines the action to be taken on input
    if num in escape_word:
        break
    if num in volume_calc:
        total, volumes = vol_calc(volumes, results_list, counter)
        print(f'{total} m3')
        continue
    if not num:
        print("Enter something")
        continue
    #finds continuous digits  
    regex_num = re.findall(r'[\d\.\d]+', num)
    #finds the units by extracting letters only
    unit_reg = re.search(r'[a-zA-Z]', num)
    #adds the units to a variable for unit
    if unit_reg:
        unit = unit_reg.group()
    if not unit_reg:
        continue    

    #the list of digits entered and extracted above
    num_list = [float(i) for i in regex_num]

    #function that determines the unit then determines the conversion
    #required to turn it into the opposite unit                        
    def unit_finder(num, unit):

        #list of unit different ways of spelling each    
        kilos = ["kgs", "k", "kg"]
        pounds = ["lbs", "l", "lb", "pounds"]
        centimeters = ["cm", "c", "centimeters"]
        inches = ["in", "i", '"']

        #assigns the conversion factor and unit to variables to be
        #called later on
        if unit in kilos:
            unit = "kgs"
            unit_convert = k_to_l
            return_unit = "lbs"

        if unit in pounds:
            unit = "lbs"
            unit_convert = l_to_k
            return_unit = "kgs"

        if unit in centimeters:
            unit = "cms"
            unit_convert = c_to_i
            return_unit = "inch"

        if unit in inches:
            unit = "inches"
            unit_convert = i_to_c
            return_unit = "cms"
        #returns the variable with the correct factor, unit and the converted unit   
        return return_unit, unit_convert, unit

    #function that does the conversion math then adds each converted
    #number to a new list    
    def calc(return_unit, unit_convert, unit, num_list):
        results_list=[]
        for i in range(len(num_list)):
            results_list.append(round(float(num_list[i] * unit_convert), 1))

        return return_unit, unit, results_list

    #calculates the volume and adds it to a dictionary with carton
    #identifier as the key
    def vol_calc(volumes, results_list, counter):
        total = 1
        for i in range(0, len(results_list)):
            total *= results_list[i]
        total = round(total/1000000, 2)
        volumes.update({counter:total})
        return total, volumes    

    #takes the results list and adds it to a dictionary with carton
    #identifier as the key    
    def add_to_shpmnt(shpmnt, results_list, counter):
        shpmnt.update({counter:results_list})
        return shpmnt

    #function that updates all the ditionaries with the newest 
    #values    
    def cont_prompt_a(shpmnt, volumes, results_list, counter):
        shpmnt = add_to_shpmnt(shpmnt, results_list, counter)
        total, volumes = vol_calc(volumes, results_list, counter)
        return shpmnt, volumes

    #call to function for conversion and unit finder                      
    return_unit, unit_convert, unit = unit_finder(num_list, unit)
    return_unit, unit, results_list = calc(return_unit, unit_convert, unit, num_list)

    #prints the conversion results
    for i in range(len(results_list)):
        print(f'{num_list[i]} {unit} is {results_list[i]} {return_unit}')

    #call to function to add all values to dictionaries
    shpmnt, volumes = cont_prompt_a(shpmnt, volumes, \
                                    results_list, counter)
    #add weight value to a new dictionary
    weight = input("\nEnter the weight: ")
    if not weight:
        continue
    weights.update({counter:weight})
    if weight in escape_word:
        break

    #prints all the dictionaries in a organized fashion
    for key, value in shpmnt.items():
        print(f'\n\tCarton #: {key}')
        print(f'\tVolume: {volumes[key]} m3')
        if weights:
            print(f'\tWeight:  {weights[key]} lbs')
        print(f'\tDims: {shpmnt[key]} cm')    
\$\endgroup\$
6
\$\begingroup\$

Quick local improvements:

redundant conversions:

k_to_l = float(2.2046226218)

is a useless float conversion when you can do:

k_to_l = 2.2046226218

why 2 if there?

if unit_reg:
    unit = unit_reg.group()
if not unit_reg:
    continue    

when you can use else (since unit_reg doesn't change):

if unit_reg:
    unit = unit_reg.group()
else:
    continue    

use list comprehensions and avoid list indices when you can:

    results_list=[]
    for i in range(len(num_list)):
        results_list.append(round(float(num_list[i] * unit_convert), 1))

can be written as

results_list = [round(float(value * unit_convert), 1) for value in num_list]

no indices (again):

    for i in range(0, len(results_list)):
        total *= results_list[i]

rewrite as (no built-in function to perform multiply like sum does with +, but reduce is also overkill) so:

    for value in results_list:
        total *= value

really avoid indices when you can:

#prints the conversion results
for i in range(len(results_list)):
    print(f'{num_list[i]} {unit} is {results_list[i]} {return_unit}')

rewritten using zip to interlace both lists as 2 variables:

for num,result in zip(num_list,results_list):
    print(f'{num} {unit} is {results} {return_unit}')

don't update your dictionaries with a dictionary of 1 element:

shpmnt.update({counter:results_list})

is better like:

shpmnt[counter] = results_list

and to conclude, the clunky part with the if on units could be better if you built a dictionary with all shortened ways of writing each unit associated to the triplet (unit, conversion function, return type).

Initialize (once) like this:

units_dict = {}
kilos = ["kgs", "k", "kg"]
pounds = ["lbs", "l", "lb", "pounds"]
centimeters = ["cm", "c", "centimeters"]
inches = ["in", "i", '"']
for k in kilos:
    units_dict[k] = ("kgs",k_to_l,"lbs")
for p in pounds:
    units_dict[p] = ("lbs",l_to_k,"kgs")
for c in centimeters:
    units_dict[c] = ("cms",c_to_i,"inch")
for i in inches:
    units_dict[i] = ("inches",i_to_c,"cms")

Now use in your program in a single line:

unit,unit_convert,return_unit = units_dict[unit]

the dictionary returns the triplet and unpacks into the left side variables.

\$\endgroup\$
  • \$\begingroup\$ This is awesome. I will be working on all your suggestions today. Thank you! \$\endgroup\$ – Tim B. May 17 '18 at 15:08
  • \$\begingroup\$ Is this correct: results_list = [round(float(value * unit_convert), 1) for value in num_list] \$\endgroup\$ – Tim B. May 17 '18 at 18:06
  • \$\begingroup\$ good catch. Edited. you're getting the hang of it :) \$\endgroup\$ – Jean-François Fabre May 17 '18 at 18:13
  • \$\begingroup\$ The zip function is awesome, didn't know about that one! \$\endgroup\$ – Tim B. May 17 '18 at 18:15

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.