Simple MAC converter

Question

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)

Answer 1

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))

Answer 2

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 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.)

Answer 3

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).