# Simple MAC converter

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)

#Open file with MAC addresses and convert them
with open('mac.txt', 'r') as f:
mac_conv(mac)


Your code's pretty good. The only thing I'd change is to reduce the amount of lines. To do this I would base my algorithm around the use of a list comprehension.

1. Split MAC into segments.
2. Slice segments into a three item list. (Like gr1 without the join)
3. Perform a list comprehension to join the sliced segments together.
4. Join the new three item list together.
5. Return it.
segments = mac.split(sep)
groups = [segments[0:2], segments[2:4], segments[4:]]
a = [''.join(group) for group in groups]
mac = '.'.join(a)
return mac


I'd then join the last three lines into one. As return and '.'.join are easy to understand in a single one-liner.

Other than the above function, you don't need to use f.readlines() you can just use f. And you should always use four spaces.

This can result in:

def mac_conv(mac, sep='-'):
segments = mac.split(sep)
groups = [segments[0:2], segments[2:4], segments[4:]]
return '.'.join(''.join(group) for group in groups)

with open('mac.txt', 'r') as f:
for mac in f:
print(mac_conv(mac))

• your answer seems more understandable for me at the moment thanks. – Erjol Bane Aug 12 '16 at 14:44
• Well, either four spaces or a tab. But no mixing both!. – Kroltan Aug 13 '16 at 1:10

First of all, you should be using 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

#Open file with MAC addresses and convert them
with open('mac.txt', 'r') as f:
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.)

• why not use tuple unpacking: return "{}{}.{}{}.{}{}".format(*mac.split(sep)) – Graipher Aug 12 '16 at 14:15
• thanks for your answer. I don't understand how does this part ""{0[0]}{0[1]}.{0[2]}{0[3]}.{0[4]}{0[5]}".format(mac_parts)" work though, because of my python knowledge so far. – Erjol Bane Aug 12 '16 at 14:50
• @ErjolBane take a look at pyformat.info, specifically this section. – RoadieRich Aug 12 '16 at 15:02

### Overall

Code seems fine - it works and is easy to comprehend.

### Reusability

You print the result instead of return-ing it. If you wanted to do something else with MAC in that format (like printing it as a part of some string), what would you do?

### Naming

There may be other formats for MAC addresses. If someone had successfully guessed that mac_conv converts MAC address, they'd still have to guess the format. mac_to_cisco_format is long, mac_to_cisco may be alright.

separator instead of sep may help a little bit to someone who only sees function's signature (as in many IDEs).

### Documentation

#Coverts MAC Addresses into cisco format should be a docstring instead.

#print final MAC address is just a visual garbage - next line is self-descriptive enough.

### Algorithm

You assume some format of input data, while it's not spelled-out explicitly. While you could handle that with naming/documentation, you can change your algorithm to convert MACs from several formats to CISCO one - simply remove separators from strings (multiple str.replace-s will do) and then slice the result as you wish.

In fact, you could accept desired format as a parameter (instead of your sep).