# length unit converter

I'm trying to make a imperial/metric length unit converter in Python. Here is my full code:

Metric = MM, CM, DM ,M, DAM, HM, KM = ('mm', 'cm','dm','m', 'dam' ,'hm', 'km')
Imperial = IN, FT, YD, MI, NM, barleycorn = ('in', 'ft', 'yd', 'mi', 'nm', 'barleycorn')
Vaildunit = Metric + Imperial

# defining
def valid(unit):
if unit not in Vaildunit:
print('\nnot in list. the unit need to be in ' + ' '.join(Vaildunit))
return False
return True

def convert2(val, unit_in, unit_out):
sol = 0.0
n = 0.0
standard = {MM: 0.001, CM: 0.01, DM : 0.1, M: 1.0, DAM : 10.0, HM : 100.0, KM: 1000.0}

# imperial to imperial
if unit_in in Metric and unit_out in Metric:
sol = val*(standard[unit_in]/standard[unit_out])

# imperial to imperial
if unit_in in Imperial and unit_out in Imperial:
if unit_in in [FT] and unit_out in [IN]:
sol = val * 12
if unit_in in [IN] and unit_out in [FT]:
sol = val / 12
if unit_in in [FT] and unit_out in [MI]:
sol = val / 5280
if unit_in in [MI] and unit_out in [FT]:
sol = val * 5280
if unit_in in [YD] and unit_out in [IN]:
sol = val * 36
if unit_in in [IN] and unit_out in [YD]:
sol = val / 36
if unit_in in [FT] and unit_out in [YD]:
sol = val * 3
if unit_in in [YD] and unit_out in [FT]:
sol = val / 3
if unit_in in [NM] and unit_out in [IN]:
sol = val * 72913.4
if unit_in in [NM] and unit_out in [FT]:
sol = val * 6076.12
if unit_in in [NM] and unit_out in [YD]:
sol = val * 2025.32
if unit_in in [NM] and unit_out in [MI]:
sol = val * 1.15078
if unit_in in [barleycorn] and unit_out in [IN]:
sol = val / 3
if unit_in in [IN] and unit_out in [barleycorn]:
sol = val * 3

# Metric to imperial
if unit_in in Metric and unit_out in Imperial:
if unit_out in [FT]:
n = val*3.28084
if unit_out in [IN]:
n = val*39.37008
if unit_out in [MI]:
n = val/1609.35
if unit_out in [YD]:
n = val*1.0936132983
if unit_out in [NM]:
n = val / 0.000539957
sol = n * standard[unit_in]

# Imperial to Metric
if unit_in in Imperial and unit_out in Metric:
standard = {MM: 0.001, CM: 0.01, DM : 0.1, M: 1.0, DAM : 10.0, HM : 100.0, KM: 1000.0}
if unit_in in [FT]:
n = val / 3.28084
if unit_in in [IN]:
n = val / 39.37008
if unit_in in [MI]:
n = val * 1609.34
if unit_in in [YD]:
n = val / 1.0936132983
if unit_in in [NM]:
n = val * 0.000539957
sol = n / standard[unit_out]
return sol

# check the list
def isfloat(string):
try:
float(string)
return True
except ValueError:
return False


In the imperial-to-imperial part, the code is too long to use. I would like to shorten that part of the code.

• too long to use? Commented Feb 8, 2023 at 13:53
• I mean its too long so it seems very unclear Commented Feb 8, 2023 at 14:03

There's no point to keeping separate variables for Metric, MM, etc. This can reduce to one dictionary.

Vaildunit should be spelled VALID_UNIT - capitals because it's a global constant.

Don't write any if statements for this purpose.

Don't bother writing logic specifically to convert from yards to inches, etc. Just convert everything referenced to a single base unit, metres.

Don't write an equality test like this: in [NM]; instead, just compare == NM.

Your conversion quantities such as 1.0936132983 lose accuracy because they aren't expressed in terms of fundamental conversion definitions. I demonstrate the alternative below.

## Suggested

UNITS = {
'mm': 0.001,
'cm': 0.01,
'dm': 0.1,
'm': 1,
'dam': 10,
'hm': 100,
'km': 1000,
'barleycorn': 0.0254/3,
'in': 0.0254,
'ft': 0.3048,
'yd': 0.9144,
'mi': 1609.344,
'nm': 1852,
}

def convert(val: float, unit_in: str, unit_out: str) -> float:
return val * UNITS[unit_in] / UNITS[unit_out]


If you think about it, the whole idea is only about multiplying your input value by some ratio that is determined by units of your output and input. You can define this nicely as data structure, something like:

unit_to_unit_ratios = {
MM: {
MM: 1,
IN: 0.0393701,
YE:...
},
FT: {
...
}
}


Then your final code is simply just:

unit_to_unit_ratios[input_unit][output_unit] * input_value


You can optimize this, for example by checking if input and output units are equal and then just return input_value so that "1" value doesn't have to be in the dictionary. Depends if you want to have longer code or longer data structure. Both has advantages and disadvantages.

And you don't need to make any kind of difference between imperial and metric units.

• I do not recommend this approach. The number of conversions you will need to express scales with O(n²). Instead prefer a system where every unit is only defined against a base unit like metres. Commented Feb 8, 2023 at 15:12
• Good point there!
– K.H.
Commented Feb 8, 2023 at 15:25

The keys of the dictionary ( Imperial_to_Imperial ) are tuples that represent the input and output units, and the values are the conversion factors.

With use of dictionary you can reduce multiple if else syntax with simple dictionary object.

You can try shorter version of code :

Imperial_to_Imperial = {
(FT, IN): 12,
(IN, FT): 1 / 12,
(FT, MI): 1 / 5280,
(MI, FT): 5280,
(YD, IN): 36,
(IN, YD): 1 / 36,
(FT, YD): 3,
(YD, FT): 1 / 3,
(NM, IN): 72913.4,
(NM, FT): 6076.12,
(NM, YD): 2025.32,
(NM, MI): 1.15078,
(barleycorn, IN): 1 / 3,
(IN, barleycorn): 3,
}


Usecase :

sol = val * Imperial_to_Imperial[(unit_in, unit_out)]

• Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. Commented Feb 9, 2023 at 14:01
• @TobySpeight The user made an indirect observation that the code is too verbose. Commented Feb 9, 2023 at 15:16
• @TobySpeight He improved a part of the code. This is a useful feedback, in my humble opinion Commented Feb 9, 2023 at 18:06
• @Billal - it may be improved, but without any explanation of why it's better, it's just an independent implementation that is not a review. Commented Feb 9, 2023 at 19:49