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

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