HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Simple MAC converter

Submitted by: @import:stackexchange-codereview··
0
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 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.