Readability Counts

/ Python & Django Trainer / @treyhunner

“Readability is the ease with which a reader can understand a written text.” —Wikipedia

Why does readability matter?

  • Readability is about making our lives easier
  • Code is read more often than it is written
  • Better readability means easier maintainability
  • Readable code makes on-boarding easier

We're not discussing

  • Writability
  • Performance
  • Python-speaking robots

We will discuss

  • Whitespace, line breaks, and code structure
  • Giving concepts a name
  • Choosing the right construct

Structuring code

  • Line length: number of characters in one line of text
  • Line length is a human limitation, not technical one
  • Focus on readability when wrapping lines, not line length

Line Breaks


employee_hours = (schedule.earliest_hour for employee in
                  self.public_employees for schedule in
                  employee.schedules)
return min(h for h in employee_hours if h is not None)
          


employee_hours = (
    schedule.earliest_hour
    for employee in self.public_employees
    for schedule in employee.schedules
)
return min(
    hour
    for hour in employee_hours
    if hour is not None
)

Regular Expressions


def is_valid_uuid(uuid):
    """Return True if given variable is a valid UUID."""
    return bool(re.search(r'^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}'
                          r'-[0-9a-f]{4}-[0-9a-f]{12}$',
                          uuid, re.IGNORECASE))
          


def is_valid_uuid(uuid):
    """Return True if given variable is a valid UUID."""
    UUID_RE = re.compile(r'''
        ^
        [0-9a-f] {8}    # 8 hex digits
        -
        [0-9a-f] {4}    # 4 hex digits
        -
        [0-9a-f] {4}    # 4 hex digits
        -
        [0-9a-f] {4}    # 4 hex digits
        -
        [0-9a-f] {12}   # 12 hex digits
        $
    ''')
    return bool(UUID_RE.search(uuid, re.IGNORECASE | re.VERBOSE))

Function Calls


default_appointment = models.ForeignKey(
    'AppointmentType', null=True, on_delete=models.SET_NULL,
    related_name='+')
          

default_appointment = models.ForeignKey('AppointmentType',
                                        null=True,
                                        on_delete=models.SET_NULL,
                                        related_name='+')
          

default_appointment = models.ForeignKey(
    'AppointmentType',
    null=True,
    on_delete=models.SET_NULL,
    related_name='+')
          


default_appointment = models.ForeignKey(
    othermodel='AppointmentType', null=True,
    on_delete=models.SET_NULL, related_name='+')
          

default_appointment = models.ForeignKey(othermodel='AppointmentType',
                                        null=True,
                                        on_delete=models.SET_NULL,
                                        related_name='+')
          

default_appointment = models.ForeignKey(
    othermodel='AppointmentType',
    null=True,
    on_delete=models.SET_NULL,
    related_name='+')
          

default_appointment = models.ForeignKey(
    othermodel='AppointmentType',
    null=True,
    on_delete=models.SET_NULL,
    related_name='+')
          


default_appointment = models.ForeignKey(
    othermodel='AppointmentType',
    null=True,
    on_delete=models.SET_NULL,
    related_name='+'
)
          


default_appointment = models.ForeignKey(
    othermodel='AppointmentType',
    null=True,
    on_delete=models.SET_NULL,
    related_name='+',
)

PEP 8

  • PEP 8 is the Python style guide
  • Read PEP 8 every 6 months
  • PEP 8 is not your project's style guide
  • PEP 8 is just a sane starting point

Recap: code structure

  • Keep your text width narrow
  • Do not rely on automatic line wrapping
  • Insert line breaks with readability in mind
  • Document your code styles and stick to them

Naming things

  • Important concepts deserve names
  • Naming things is hard because describing things is hard
  • A name should fully & accurately describe its subject
  • Don't be afraid of using long variable names

Better Names


sc = {}
for i in csv_data:
    sc[i[0]] = i[1]
          


state_capitals = {}
for i in capitals_csv_data:
    state_capitals[i[0]] = i[1]
          


state_capitals = {}
for s, c, *_ in capitals_csv_data:
    state_capitals[s] = c
          


state_capitals = {}
for state, capital, *_ in capitals_csv_data:
    state_capitals[state] = capital

Name All The Things


def detect_anagrams(word, candidates):
    anagrams = []
    for candidate in candidates:
        if (sorted(word.upper()) == sorted(candidate.upper())
                and word.upper() != candidate.upper()):
            anagrams.append(candidate)
          


def detect_anagrams(word, candidates):
    anagrams = []
    for candidate in candidates:
        if is_anagram(word, candidate):
            anagrams.append(candidate)
          


def is_anagram(word1, word2):
    return (sorted(word1.upper()) == sorted(word2.upper())
            and word1.upper() != word2.upper())
          


def is_anagram(word1, word2):
    word1, word2 = word1.upper(), word2.upper()
    return sorted(word1) == sorted(word2) and word1 != word2
          


def is_anagram(word1, word2):
    word1, word2 = word1.upper(), word2.upper()
    return (
        sorted(word1) == sorted(word2)  # words have same letters
        and word1 != word2  # words are not the same
    )
          


def is_anagram(word1, word2):
    word1, word2 = word1.upper(), word2.upper()
    are_different_words = (word1 != word2)
    have_same_letters = (sorted(word1) == sorted(word2))
    return have_same_letters and are_different_words
          


def detect_anagrams(word, candidates):
    anagrams = []
    for candidate in candidates:
        if is_anagram(word, candidate):
            anagrams.append(candidate)


def is_anagram(word1, word2):
    word1, word2 = word1.upper(), word2.upper()
    are_different_words = word1 != word2
    have_same_letters = sorted(word1) == sorted(word2)
    return have_same_letters and are_different_words

So Many Functions


def update_appointment_types(self):
    """Delete/make appt. types and set default appt. type"""

    self.appt_types.exclude(specialty=self.specialty).delete()

    new_types = self.specialty.appt_types.exclude(agent=self)
    self.appt_types.bulk_create(
        AppointmentType(agent=self, appointment_type=type_)
        for type_ in new_types
    )

    old_default_id = self.default_appt_id
    self.default_appt_type = self.specialty.default_appt_type
    if self.default_appt_type.id != old_default_id:
        self.save(update_fields=['default_appt_type'])
          


def update_appointment_types(self):
    """Delete/make appt. types and set default appt. type"""

    self.appt_types.exclude(specialty=self.specialty).delete()

    new_types = self.specialty.appt_types.exclude(agent=self)
    self.appt_types.bulk_create(
        AppointmentType(agent=self, appointment_type=type_)
        for type_ in new_types
    )

    old_default_id = self.default_appt_id
    self.default_appt_type = self.specialty.default_appt_type
    if self.default_appt_type.id != old_default_id:
        self.save(update_fields=['default_appt_type'])
          


def update_appointment_types(self):
    """Delete/make appt. types and set default appt. type"""

    # Delete appointment types for specialty besides current one
    self.appt_types.exclude(specialty=self.specialty).delete()

    # Create new appointment types based on specialty (if needed)
    new_types = self.specialty.appt_types.exclude(agent=self)
    self.appt_types.bulk_create(
        AppointmentType(agent=self, appointment_type=type_)
        for type_ in new_types
    )

    # Set default appointment type based on specialty
    old_default_id = self.default_appt_id
    self.default_appt_type = self.specialty.default_appt_type
    if self.default_appt_type.id != old_default_id:
        self.save(update_fields=['default_appt_type'])
          


def update_appointment_types(self):
    """Delete/make appt. types and set default appt. type"""

    # Delete appointment types for specialty besides current one
    self.appt_types.exclude(specialty=self.specialty).delete()

    # Create new appointment types based on specialty (if needed)
    new_types = self.specialty.appt_types.exclude(agent=self)
    self.appt_types.bulk_create(
        AppointmentType(agent=self, appointment_type=type_)
        for type_ in new_types
    )

    # Set default appointment type based on specialty
    old_default_id = self.default_appt_id
    self.default_appt_type = self.specialty.default_appt_type
    if self.default_appt_type.id != old_default_id:
        self.save(update_fields=['default_appt_type'])
          


def update_appointment_types(self):
    """Delete/make appt. types and set default appt. type"""

    # Delete appointment types for specialty besides current one
    self.appt_types.exclude(specialty=self.specialty).delete()

    # Create new appointment types based on specialty (if needed)
    new_types = self.specialty.appt_types.exclude(agent=self)
    self.appt_types.bulk_create(
        AppointmentType(agent=self, appointment_type=type_)
        for type_ in new_types
    )

    # Set default appointment type based on specialty
    old_default_id = self.default_appt_id
    self.default_appt_type = self.specialty.default_appt_type
    if self.default_appt_type.id != old_default_id:
        self.save(update_fields=['default_appt_type'])
          


def _delete_stale_appointment_types(self):
    """Delete appointment types for specialties besides ours"""
    self.appt_types.exclude(specialty=self.specialty).delete()

def _create_new_appointment_types(self):
    """Create new appointment types based on specialty if needed"""
    new_types = self.specialty.appt_types.exclude(agent=self)
    self.appt_types.bulk_create(
        AppointmentType(agent=self, appointment_type=type_)
        for type_ in new_types
    )

def _update_default_appointment_type(self):
    """Set default appointment type based on specialty"""
    old_default_id = self.default_appt_id
    self.default_appt_type = self.specialty.default_appt_type
    if self.default_appt_type.id != old_default_id:
        self.save(update_fields=['default_appt_type'])

          


def update_appointment_types(self):
    """Delete/make appt. types and set default appt. type"""
    self._delete_stale_appointment_types()
    self._create_new_appointment_types()
    self._update_default_appointment_type()

Recap: naming things

  • Use whole words for variable names
  • Try to use tuple unpacking instead of index lookups
  • Read your code aloud to test whether it's descriptive
  • Try converting comments to better variable names
  • Create names for unnamed concepts/code
  • In general, try to make your code self-documenting

Programming idioms

  • Special purpose constructs can reduce complexity
  • When possible, use constructs with specific intent

Clean Up


db = DBConnection("mydb")
try:
    records = db.query_all()
finally:
    db.close()
          


class connect:
    def __init__(self, path):
        self.connection = DBConnection(path)

    def __enter__(self):
        return self.connection

    def __exit__(self):
        self.close()
          


with connect("mydb") as db:
    db.query_all()
          


from contextlib import closing

with closing(DBConnection("mydb")) as db:
    db.query_all()

Lists from lists


employees  = []
for calendar, availabilities in calendar_availabilities:
    if availabilities:
        employees.append(calendar.employee)
          


employees  = []
for calendar, availabilities in calendar_availabilities:
    if availabilities:
        employees.append(calendar.employee)
          


employees = [
    calendar.employee
    for (calendar, availabilities) in
    calendar_availabilities
    if availabilities
]

Operator Overloading


class ShoppingCart:
    def contains(self, product):
        """Return True if cart contains the product."""

    def add(self, product, quantity):
        """Add the quantity of a product to the cart."""

    def remove(self, product):
        """Completely remove a product from the cart."""

    def set(self, product, quantity):
        """Set the quantity of a product in the cart."""

    @property
    def count(self):
        """Return product count in cart, ignoring quantities."""

    @property
    def is_empty(self):
        """Return True if cart is empty."""

BeforeAfter
cart.contains(item)item in cart
cart.set(item, q)cart[item] = q
cart.add(item, q)cart[item] += q
cart.remove(item)del cart[item]
cart.countlen(cart)
cart.is_emptynot cart 


class ShoppingCart:
    def __contains__(self, product):
        """Return True if cart contains the product."""

    def __setitem__(self, product, quantity):
        """Set the quantity of a product in the cart."""

    def __delitem__(self, product):
        """Completely remove a product from the cart."""

    def __len__(self):
        """Return product count in cart, ignoring quantities."""

    def __bool__(self):
        """Return True if cart is non-empty."""

Abstract Base Classes

  • collections.UserList: make custom list
  • collections.UserDict: make custom dictionary
  • collections.UserString: make custom string

Shared data


def get_connection(host, username, password):
    """Initialize IMAP server and login"""
    server = IMAP4_SSL(host)
    server.login(username, password)
    server.select("inbox")
    return server

def close_connection(server):
    server.close()
    server.logout()

def get_message_uids(server):
    """Return unique identifiers for each message"""
    return server.uid("search", None, "ALL")[1][0].split()

def get_message(server, uid):
    """Get email message identified by given UID"""
    result, data = server.uid("fetch", uid, "(RFC822)")
    (_, message_text), _ = data
    message = Parser().parsestr(message_text)
    return message
          


def get_connection(host, username, password):
    """Initialize IMAP server and login"""
    server = IMAP4_SSL(host)
    server.login(username, password)
    server.select("inbox")
    return server

def close_connection(server):
    server.close()
    server.logout()

def get_message_uids(server):
    """Return unique identifiers for each message"""
    return server.uid("search", None, "ALL")[1][0].split()

def get_message(server, uid):
    """Get email message identified by given UID"""
    result, data = server.uid("fetch", uid, "(RFC822)")
    (_, message_text), _ = data
    message = Parser().parsestr(message_text)
    return message
          


class IMAPChecker:
    def __init__(self, host):
        """Initialize IMAP email server with given host"""
        self.server = IMAP4_SSL(host)

    def authenticate(self, username, password):
        """Authenticate with email server"""
        self.server.login(username, password)
        self.server.select("inbox")

    def quit(self):
        self.server.close()
        self.server.logout()

    def get_message_uids(self):
        """Return unique identifiers for each message"""
        return self.server.uid("search", None, "ALL")[1][0].split()

    def get_message(self, uid):
        """Get email message identified by given UID"""
        result, data = self.server.uid("fetch", uid, "(RFC822)")
        (_, message_text), _ = data
        message = Parser().parsestr(message_text)
        return message

Recap: programming idioms

  • Think about using a context manager when you notice duplicate code wrapped around other code
  • When making one list from another, use a comprehension
  • Don't be afraid to use dunder methods
  • Classes are good for bundling up code and data

Readability checklist

  1. Can I modify line breaks to improve clarity?
  2. Can I create a variable name for unnamed code?
  3. Can I add a comment to improve clarity?
  4. Can I turn a comment into a better variable name?
  5. Can I use a more specific programming construct?
  6. Have I stated detailed preferences in a style guide?

My style guide

http://trey.io/style-guide

More talks to watch

@treyhunner
http://WeeklyPython.Chat
Python & Django Trainer for hire