Request a short code review

[james at reggieband.com]

Thank you all for posting insightful and useful comments.

Here is what I have based on your input:


def output_random_lesson_of_type(self, lesson_type=None):
    """Output a lesson of a specific type.
       If no type is passed in then output all types."""
    lessons = self.lesson_data["lessons"]
    if lesson_type:
        lessons = [x for x in lessons if x["type"] == lesson_type]
    rand_lesson = choice(lessons)
    inputs = self.create_lesson_inputs(rand_lesson)
    print rand_lesson["output"] % inputs
    return rand_lesson, inputs

Changes:
 - generator expression instead of filter
 - removed keyword 'type' in favor of lesson_type
 - wrap to 78 columns
 - remove error-check -- allow it to fail in the choice statement.
   I've decided to make a valid lesson_type an invariant anyway
   so the appropriate thing would be an assert - but I'll settle for
   a throw from choice if it receives an empty list
 - moved self.output_random logic into this function

The lesson data is loaded from YAML which explains why it is inside a
dictionary instead of a class.  I am currently exploring ways to allow
me to easily switch my data providers between SQLAlchemy and YAML.

Cheers again.  This is very valuable for me.  I am a true believer
that if one takes care in the smallest of methods then the larger code-
base will be all the better.

James.