[EMAIL PROTECTED] wrote: >> I am not necessarily looking to make the code shorter or more >> functional or anything in particular. However if you spot something >> to improve then I am happy to learn. > > To give an example of what I mean I have already altered the code: > > def output_random_lesson_of_type(self, type=None): > """Output a lesson of a specific type. > If no type is passed in then output any type."""
"any" type? Perhaps you mean all types. > output_lessons = self.lesson_data["lessons"] > if type: > filtered_lessons = filter(lambda x: x["type"] == type, > self.lesson_data["lessons"]) filter/map/reduce/lambda is not to everyone's taste; consider using a list comprehension: filtered_lessons = [x for x in self.lesson_data["lessons"] if x["type"] == type] Now you need to go up a level ... when you find yourself using dictionaries with constant string keys that are words, it's time to consider whether you should really be using classes: filtered_lessons = [x for x in self.lesson_data.lessons if x.type == type] > if filtered_lessons: > output_lessons = filtered_lessons > else: > print "Unable to find lessons of type %s." % type So the error action is to print a vague message on stdout and choose from all lessons? > return self.output_random(output_lessons) > > Changes: > - Respected a column width of 80 If you really care about people who are still using green-screen terminals or emulations thereof, make it < 80 -- some (most? all?) terminals will produce an annoying blank line if the text is exactly 80 bytes long. > - Created the output_lessons variable, assigned it to a default. > This remove an else statement and reduced the call to > self.output_random to a single instance in the return statement ... and also makes folk who are interested in what happens in the error(?) case read backwards to see what lessons will be used. HTH, John -- http://mail.python.org/mailman/listinfo/python-list
