Request a short code review

[Scott David Daniels]

james at reggieband.com wrote:
> Here is a method I came across that I would like to clean up:
> 
> 
> 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."""
>     if type:
>         filtered_lessons = filter(lambda x: x["type"] == type,
> self.lesson_data["lessons"])
>         if filtered_lessons:
>             lesson = self.output_random(filtered_lessons)
>         else:
>             print "Unable to find lessons of type %s." % type
>     else:
>         lesson = self.output_random(self.lesson_data["lessons"])
>     return lesson

Simplest: Just let the error float up to where it is caught

import random

def output_random_lesson_of_type(self, type=None):
     """Output a lesson of a specific type - None mans any type."""
     lessons = self.lesson_data["lessons"]
     if type is not None:
         lessons = [x for x in lessons if x["type"] == type]
     return random.choice(lessons)

Or, if you really want to print your message, change that last line to:

     try:
         return random.choice(lessons)
     except IndexError:
         print "Unable to find lessons of type %s." % type
         raise


-Scott David Daniels
Scott.Daniels at Acm.Org