patternpythonMinor
Simple MAC converter
Viewed 0 times
macsimpleconverter
Problem
I'm new to Python and experimenting with some simple scripts. I would appreciate if you could give me some suggestions on how the following code can be improved or made simpler or anything in general.
#Coverts MAC Addresses into cisco format
def mac_conv(mac, sep='-'):
#split mac address into 3 groups
splitted_mac = mac.split(sep)
gr1 = ''.join(splitted_mac[0:2])
gr2 = ''.join(splitted_mac[2:4])
gr3 = ''.join(splitted_mac[4:])
#join groups into a single MAC
mac = gr1, gr2, gr3
final_mac = '.'.join(mac)
#print final MAC address
print(final_mac)
#Open file with MAC addresses and convert them
with open('mac.txt', 'r') as f:
for mac in f.readlines():
mac_conv(mac)Solution
First of all, you should be using
I would also avoid reusing variables, especially if its a function argument (
If I was writing this, however, I would probably do it in just a couple of lines, as you know exactly how the input and output strings are going to be formatted:
If this was going into a product, I would probably add some error handling too, to allow for input that isn't a mac address - too short, too long, wrong separators, alternate groupings (a cisco-formatted address is still a valid mac address, but this function won't handle it.)
return rather than print to get a value out of a function, otherwise you'll have problems when you try to expand the program.def mac_conv(mac, sep='-'):
#etc
#return final MAC address
return final_mac
#Open file with MAC addresses and convert them
with open('mac.txt', 'r') as f:
for mac in f.readlines():
print(mac_conv(mac))I would also avoid reusing variables, especially if its a function argument (
mac). You can actually eliminate that usage of the variable all together:#join groups into a single MAC
final_mac = '.'.join([gr1, gr2, gr3])If I was writing this, however, I would probably do it in just a couple of lines, as you know exactly how the input and output strings are going to be formatted:
def mac_conv(mac, sep='-'):
mac_parts = mac.split(sep)
return "{0[0]}{0[1]}.{0[2]}{0[3]}.{0[4]}{0[5]}".format(mac_parts)If this was going into a product, I would probably add some error handling too, to allow for input that isn't a mac address - too short, too long, wrong separators, alternate groupings (a cisco-formatted address is still a valid mac address, but this function won't handle it.)
Code Snippets
def mac_conv(mac, sep='-'):
#etc
#return final MAC address
return final_mac
#Open file with MAC addresses and convert them
with open('mac.txt', 'r') as f:
for mac in f.readlines():
print(mac_conv(mac))#join groups into a single MAC
final_mac = '.'.join([gr1, gr2, gr3])def mac_conv(mac, sep='-'):
mac_parts = mac.split(sep)
return "{0[0]}{0[1]}.{0[2]}{0[3]}.{0[4]}{0[5]}".format(mac_parts)Context
StackExchange Code Review Q#138527, answer score: 5
Revisions (0)
No revisions yet.