How to make python code less ugly

Question

First of all, python is an amazing language. This is my first project using python, and I have already made ridiculous progress.

This code below cannot be the best way to do this. What is the most idiomatic way to write a class definition?

class Course: crn = course = title = tipe = cr_hours = seats = instructor = days = begin = end = location = exam = "" def __init__(self, pyQueryRow): self.crn = Course.get_column(pyQueryRow, 0) self.course = Course.get_column(pyQueryRow, 1) self.title = Course.get_column(pyQueryRow, 2) self.tipe = Course.get_column(pyQueryRow, 3) self.cr_hours = Course.get_column(pyQueryRow, 4) self.seats = Course.get_column(pyQueryRow, 5) self.instructor = Course.get_column(pyQueryRow, 6) self.days = Course.get_column(pyQueryRow, 7) self.begin = Course.get_column(pyQueryRow, 8) self.end = Course.get_column(pyQueryRow, 9) self.location = Course.get_column(pyQueryRow, 10) self.exam = Course.get_column(pyQueryRow, 11) def get_column(row, index): return row.find('td').eq(index).text() 

Thanks!

Answer 1

 def__init__(self, pyQueryRow): for i,attr in enumerate("crn course title tipe cr_hours seats instructor" " days begin end location exam".split()): setattr(self, attr, self.get_column(pyQueryRow, i)) 

This method avoids multiple calls to self.get_column

 def__init__(self, pyQueryRow): attrs = ("crn course title tipe cr_hours seats instructor" " days begin end location exam".split()) values = [td.text for td in pyQueryRow.find('td')] for attr, value in zip(attrs, values): setattr(self, attr, value) 

Answer 2

Personally, I would use a dictionary to map properties to column numbers:

 class Course: crn = course = title = tipe = cr_hours = seats = instructor = days = begin = end = location = exam = "" def __init__(self, pyQueryRow): course_row_mapping = { 'crn' : 0, 'course' : 1, 'title' : 2, 'tipe' : 3, # You probably mean "type"? 'cr_hours' : 4, 'seats' : 5, 'instructor' : 6, 'days' : 7, 'begin' : 8, 'end' : 9, 'location' : 10, 'exam' : 11, } for name, col in course_row_mapping.iteritems(): setattr(self, name, Course.get_column(pyQueryRow, col)) def get_column(row, index): return row.find('td').eq(index).text() 

Answer 3

EDIT: Actually best of all:

 self.crn, self.course, self.title, self.tipe, self.cr_hours, self.seats,\ self.instructor, self.days, self.begin, self.end, self.location, self.exam = \ [pq(td).text() for td in pyQueryRow.find('td')] 

It is assumed that you have imported PyQuery as pq. This avoids the use of indexes at all.

---

 self.crn, self.course, self.title, self.tipe, self.cr_hours, self.seats,\ self.instructor, self.days, self.begin, self.end, self.location, self.exam = \ map(lambda index: get_column(pyQueryRow, index), xrange(0, 12)) 

or if you want to understand the list:

 self.crn, self.course, self.title, self.tipe, self.cr_hours, self.seats,\ self.instructor, self.days, self.begin, self.end, self.location, self.exam = \ [get_column(pyQueryRow, index) for index in xrange(0, 12)] 

I don’t know if they are the most idiomatic, but there are definitely less templates.

Also remove crn = course = . You are assigning a class, not an instance.

Answer 4

I'm not sure there is a "better" way. What you have is, of course, quite readable. If you want to avoid duplicating the code of Course.get_column , you can define a lambda for this, as in Matthew Flaschen's answer, for example.

 class Course: def __init__(self, pyQueryRow): get_column = lambda index: pyQueryRow.find('td').eq(index).text() self.crn = get_column(0) self.course = get_column(1) self.title = get_column(2) self.tipe = get_column(3) self.cr_hours = get_column(4) self.seats = get_column(5) self.instructor = get_column(6) self.days = get_column(7) self.begin = get_column(8) self.end = get_column(9) self.location = get_column(10) self.exam = get_column(11) 

Note that you do not need a line that pre-initializes all fields before "" - just setting them to __init__ fine. Edit: Actually, as Matthew says, which sets class fields, not instance fields, I completely skipped this.