Coding the Python way

Question

I've just spent the last half semester at Uni learning python. I've really enjoyed it, and was hoping for a few tips on how to write more 'pythonic' code.

This is the __init__ class from a recent assignment I did. At the time I wrote it, I was trying to work out how I could re-write this using lambdas, or in a neater, more efficient way, but ran out of time.

def __init__(self, dir):

    def _read_files(_, dir, files):

        for file in files:

            if file == "classes.txt":
                class_list = readtable(dir+"/"+file)
                for item in class_list:
                    Enrol.class_info_dict[item[0]] = item[1:]
                    if item[1] in Enrol.classes_dict:
                        Enrol.classes_dict[item[1]].append(item[0])
                    else:
                        Enrol.classes_dict[item[1]] = [item[0]]

            elif file == "subjects.txt":
                subject_list = readtable(dir+"/"+file)
                for item in subject_list:
                    Enrol.subjects_dict[item[0]] = item[1]

            elif file == "venues.txt":
                venue_list = readtable(dir+"/"+file)
                for item in venue_list:
                    Enrol.venues_dict[item[0]] = item[1:]

            elif file.endswith('.roll'):
                roll_list = readlines(dir+"/"+file)
                file = os.path.splitext(file)[0]
                Enrol.class_roll_dict[file] = roll_list
                for item in roll_list:
                    if item in Enrol.enrolled_dict:
                        Enrol.enrolled_dict[item].append(file)
                    else:
                        Enrol.enrolled_dict[item] = [file]


    try:
        os.path.walk(dir, _read_files, None)
    except:
        print "There was a problem reading the directory"

As you can see, it's a little bulky. If anyone has the time or inclination, I'd really appreciate a few tips on some python best-practices.

Thanks.

Answer 1

Couple things that can clean up your code a bit:

Use the dictionary's setdefault. If the key is missing, then it sets it to the default you provide it with, then returns it. Otherwise, it just ignores the 2nd parameter and returns what was in the dictionary. This avoids the clunky if-statements.

Enrol.venues_dict.setdefault(key, []).append(file)

>>> x = {}
>>> x.setdefault(99, []).append(5) 
>>> x.setdefault(99, []).append(6)
>>> x
{99: [5, 6]}
>>> x.setdefault(100, []).append(1)
>>> x
{99: [5, 6], 100: [1]}

The other possibility is to use os.path.join to make file paths. This is safer than just doing string concatenation.

os.path.join(dir, file)

Other than that, looks good in terms of style, IMO.

Answer 2

Another important point if you want to use you script for long (some would say very long) is to not use deprecated functions in new code:

os.path.walk dissapeared in python 3.x. Now you can use os.walk instead. However os.walk is different to os.path.walk : it does not accept a processing function in its signature. So refactoring your code will imply a little bit more than changing names.

Answer 3

In addition to orangeoctopus's suggestions to use setdefault, you can refactor the if-else into a dispatcher (the typical replacement for big if-else and switch statements):

# list of 2-tuples: (bool func(string filename), handler_function)
handlers = [
  ((lambda fn: fn == "classes.txt"), HandleClasses),
  ((lambda fn: fn == "subjects.txt"), HandleSubjects),
  ((lambda fn: fn.endswith(".roll")), HandleRoll)
]

then do

for filename in files:
  for matcher, handler in handlers:
    if matcher(filename):
      handler(filename)
      break