Readability counts.

Ignacy Sokołowski

Introduction

Why the code readability is important?
How to write readable code in Python?
Examples of readable and unreadable code
How easy it is to mislead yourself and other people
How to prevent it

The talk title

>>> import this
The Zen of Python, by Tim Peters

Beautiful is better than ugly.
Explicit is better than implicit.
Simple is better than complex.
Complex is better than complicated.
Flat is better than nested.
Sparse is better than dense.
Readability counts.
Special cases aren't special enough to break the rules.
Although practicality beats purity.
Errors should never pass silently.
Unless explicitly silenced.
In the face of ambiguity, refuse the temptation to guess.
There should be one-- and preferably only one --obvious way to do it.
Although that way may not be obvious at first unless you're Dutch.
Now is better than never.
Although never is often better than *right* now.
If the implementation is hard to explain, it's a bad idea.
If the implementation is easy to explain, it may be a good idea.
Namespaces are one honking great idea -- let's do more of those!

Why

Why readability counts?

Code is read much more often than it is written
Once you read your code after a long break, you won't remember how it works
You're not the only one who will read your code
If it's hard to read, it's also hard to find bugs

Who reads your code?

You
You, after a break
Your co-workers
Reviewers
Users of your libraries

PEP8

Style Guide for Python code.

Avoid arguing which style is better
People get used to one guide -- no need to switch between different guides when switching projects
No need to change style when changing your job
Easier contribution

Naming conventions

ClassName (HTTPRequest vs HttpRequest, TwitterAPI vs TwitterApi)
CONSTANT
everything_else_lowercase: module, function, method, attribute, argument, etc.

A reader knows what to expect, they can guess the object type by its name.

Naming conventions: class

Class name should be CamelCase.

BAD:

class request(object):
    pass

response = request()  # What the user expects.
request = request()   # What the author meant.

GOOD:

class Request(object):
    pass

request = Request()

Naming conventions: function

Function or method name should be lowercase.

BAD:

def Request(url):
    pass

request = Request('http://...')   # What the user expects.
response = Request('http://...')  # What the author meant.

GOOD:

def request(url):
    pass

response = request('http://...')

Naming conventions: module

Module name should be lowercase.

BAD:

import os.Path

path = os.Path('/tmp/foo.json')  # What the user expects.
os.Path.exists('/tmp/foo.json')  # What the author meant.

GOOD:

import os.path

os.path.exists('/tmp/foo.json')

Naming conventions: constant

Constants should be UPPERCASE.

BAD:

from django.conf.settings import TimeZone

time_zone = TimeZone('UTC')  # What the user expects.
>>> TimeZone                 # What the author meant.
'UTC'

GOOD:

from django.conf.settings import TIME_ZONE

>>> TIME_ZONE
'UTC'

Naming conventions: function name

Function and method name should start with a verb, suggestive on the action.

BAD:

class JSONFile(object):

    def __init__(self, path):
        self.path = path

    def content(self):
        with open(self.path) as f:
            raw_content = f.read()
        content = json.loads(raw_content)
        return content

json_file = JSONFile('./data.json')
# User expects 'content` to be an attribute.
if json_file.content:
    title = json_file.content['title']
    body = json_file.content['body']

GOOD:

class JSONFile(object):

    def __init__(self, path):
        self.path = path

    def read(self):
        with open(self.path) as f:
            raw_content = f.read()
        content = json.loads(raw_content)
        return content

json_file = JSONFile('./data.json')
content = json_file.read()
if content:
    title = content['title']
    body = content['body']

Naming conventions: boolean

Booleans should start with is_, has_, etc.

BAD:

class User(object):

    def __init__(self, username, password, avatar):
        self.username = username
        self.password = password
        self.avatar = avatar

    def get_avatar_path(self):
        if self.avatar:
            return os.path.join('avatars', self.username + '.png')

>>> user = User('admin', 'passwd', 'me.png')  # What the user expects.
>>> user.avatar
'me.png'
>>> user.get_avatar_path()
'avatars/me.png'

Naming conventions: boolean

Booleans should start with is_, has_, etc.

BAD:

class User(object):

    def __init__(self, username, password, avatar):
        self.username = username
        self.password = password
        self.avatar = avatar

    def get_avatar_path(self):
        if self.avatar:
            return os.path.join('avatars', self.username + '.png')

>>> user = User('admin', 'passwd', avatar=True)   # What the author meant.
>>> user.avatar
True
>>> user.get_avatar_path()
'avatars/admin.png'
>>> user = User('admin', 'passwd', avatar=False)   # What the author meant.
>>> user.avatar_path
None

Naming conventions: boolean

Booleans should start with is_, has_, etc.

GOOD:

class User(object):

    def __init__(self, username, password, has_avatar):
        self.username = username
        self.password = password
        self.has_avatar = has_avatar

    def get_avatar_path(self):
        if self.has_avatar:
            return os.path.join('avatars', self.username + '.png')

>>> user = User('admin', 'passwd', has_avatar=True)
>>> user.has_avatar
True
>>> user.get_avatar_path()
'avatars/admin.png'
>>> user = User('admin', 'passwd', has_avatar=False)
>>> user.get_avatar_path()
None

Name length

Names should be descriptive but not too long.

BAD:

class ArticleTitleCanNotBeLongerThan100CharactersError(Exception):
    pass

from project.blog.exceptions import ArticleTitleCanNotBeLongerThan100CharactersError

GOOD:

class TitleLengthError(Exception):
    """Raised when article title is longer than 100 characters."""

from project.blog.exceptions import TitleLengthError
raise TitleLengthError("Title longer than 100 characters: %s", title)

Punctuation

Space after comma, full stop, colon, etc.

foo(1,2)  # Bad.
foo(1.2)
foo(1, 2)

foo(one, two,three, four)  # Bad.
foo(one, two.three, four)
foo(one, two, three, four)

[1,2,3,4,5,6,7]  # Bad.
[1,2,3,4.5,6,7]  # Bad.
[1, 2, 3, 4, 5, 6, 7]
[1, 2, 3, 4.5, 6, 7]

79 characters

Like reading text -- easier when the format is not too big
Small resolution, e.g. mobile
Scrolling
Even if your editor can wrap lines, it probably can't do it in a readable way
Side-by-side diffs

Indentation of continuation lines

Indentation should be always a multiple of four.

Indentation of a dictionary, list, etc.

BAD:

some_dictionary = {'ok': 200, 'not_found': 404, 'forbidden': 403,
                   'bad_request': 400, 'internal_server_error': 500}

@@ -27,7 +27,6 @@
- some_dictionary = {'ok': 200, 'not_found': 404, 'forbidden': 403,
-                    'bad_request': 400, 'internal_server_error': 500}
+ status_codes = {'ok': 200, 'not_found': 404, 'forbidden': 403,
+                 'bad_request': 400, 'internal_server_error': 500}

What was actually changed? Only the name or maybe also some key?

Indentation of a dictionary, list, etc.

BAD:

some_dictionary = {'ok': 200, 'not_found': 404, 'forbidden': 403,
                   'bad_request': 400, 'internal_server_error': 500}

@@ -27,7 +27,6 @@
- some_dictionary = {'ok': 200, 'not_found': 404, 'forbidden': 403,
-                    'bad_request': 400, 'internal_server_error': 500}
+ some_dictionary = {'ok': 200, 'unauthorized': 401, 'not_found': 404,
+                    'forbidden': 403, 'bad_request': 400,
+                    'internal_server_error': 500}

What changed? A key, a value? A new item was added?

Indentation of a dictionary, list, etc.

BETTER:

some_dictionary = {
    'ok': 200,
    'not_found': 404,
    'forbidden': 403,
    'bad_request': 400,
    'internal_server_error': 500}

@@ -27,7 +27,6 @@
- some_dictionary = {
+ status_codes = {
      'ok': 200,

Indentation of a dictionary, list, etc.

BETTER:

some_dictionary = {
    'ok': 200,
    'not_found': 404,
    'forbidden': 403,
    'bad_request': 400,
    'internal_server_error': 500}

@@ -27,7 +27,6 @@ some_dictionary = {
      'ok': 200,
+     'unauthorized': 401,
      'not_found': 404,

Indentation of a dictionary, list, etc.

BETTER:

some_dictionary = {
    'ok': 200,
    'not_found': 404,
    'forbidden': 403,
    'bad_request': 400,
    'internal_server_error': 500}

But here's a problem:

@@ -27,7 +27,6 @@ some_dictionary = {
      'bad_request': 400,
-     'internal_server_error': 500}
+     'internal_server_error': 500,
+     'conflict': 409}

Indentation of a dictionary, list, etc.

BEST:

some_dictionary = {
    'ok': 200,
    'not_found': 404,
    'forbidden': 403,
    'bad_request': 400,
    'internal_server_error': 500,
}

@@ -27,7 +27,6 @@ some_dictionary = {
     'internal_server_error': 500,
+    'conflict': 409,
 }

Indentation of a function call

BAD:

some_long_name = SomeLongClass.some_long_method_name(some_dictionary['foo'],
                                                     some_dictionary['bar'],
                                                     some_dictionary['baz'],
                                                     some_dictionary['foobar'])

Not enough space for arguments.
When a variable/class/method name changes, you have to reindent everything.

Indentation of a function call

BETTER:

some_long_name = SomeLongClass.some_long_method_name(
    some_dictionary['foo'], some_dictionary['bar'], some_dictionary['baz'],
    some_dictionary['foobar'])

If an argument changes its length, you have to change at least two lines.

Indentation of a function call

BEST:

some_long_name = SomeLongClass.some_long_method_name(
    some_dictionary['foo'],
    some_dictionary['bar'],
    some_dictionary['baz'],
    some_dictionary['foobar'])

Easier to scan all the dictionary keys.

Indentation of an `if` statement

BAD:

if (self.foo(some_argument) and
    self.bar(another_argument) and
    self.baz(third_argument) and
    self.spam(fourth_argument)):
    self.foobar(some_argument)

The last line looks like another condition for the if statement.

GOOD:

if (self.foo(some_argument) and
        self.bar(another_argument) and
        self.baz(third_argument) and
        self.spam(fourth_argument)):
    self.foobar(some_argument)

Indented twice.

Group your imports

standard library
third party
local application

Group your imports

BAD:

from django.shortcuts import render_to_response
import datetime
import requests  # Is requests in the standard library?
from django.views.generic.list_detail import object_detail
from .utils import something
import os

Group your imports

GOOD:

import datetime
import os

import requests
import sqlalchemy

from . import utils
from . import exceptions

You know what 3rd party libraries are used.

Sort imports alphabetically

It's easier to find a specific import.

import argparse
import calendar
import datetime
import logging
import io
import os
import re
import subprocess
import time
import threading

Use namespaces: never import `*`!

BAD:

from requests import *
from sqlalchemy.orm import *

# Where do these come from?
session = Session()
get(foo)

GOOD:

import requests
import sqlalchemy.orm

session = sqlalchemy.orm.Session()
requests.get(foo)

Use namespaces: import modules

It's often a better idea to import module instead of each name directly.

BAD:

from requests import get, post, Session, session, HTTPError, Response

# Somewhere at the bottom of a long module:
session = Session()
get(foo)

from requests import request

def some_django_view(request):  # Overriding 'request'.
    response = request('POST', 'http://...')

Use namespaces: import modules

It's often a better idea to import module instead of each name directly.

GOOD:

import requests

# Somewhere at the bottom of a long module:
session = requests.Session()
requests.get(foo)

import requests

def some_django_view(request):
    response = requests.request('POST', 'http://...')

import pylons

def do_something_using_pylons_config(config=None):
    config = config or pylons.config

Nesting statements in one line

[
  {"title": "foo bar", "body": "Lorem ipsum ..."},
  {"title": "bar foo", "body": "Dolor sit amet ..."}
]

BAD:

def get_first_initial_bad(file_path):
    return json.loads(open(file_path).read())[0]['title'][0].upper()

Nesting statements in one line

[
  {"title": "foo bar", "body": "Lorem ipsum ..."},
  {"title": "bar foo", "body": "Dolor sit amet ..."}
]

GOOD:

def get_first_initial_good(file_path):
    with open(file_path) as f:
        json_content = f.read()
    articles = json.loads(json_content)
    first_article = articles[0]
    title = first_article['title']
    first_letter = title[0]
    initial = first_letter.upper()
    return initial

Nesting statements in one line

[]

BAD:

Traceback (most recent call last):
  File "./initial.py", line 7, in get_first_initial_bad
    return json.loads(open(file_path).read())[0]['title'][0].upper()
IndexError: list index out of range

Which index? The first [0] or the second one?

GOOD:

Traceback (most recent call last):
  File "./initial.py", line 14, in get_first_initial_good
    first_article = articles[0]
IndexError: list index out of range

Nesting statements in one line

[{"title": []}]

BAD:

Traceback (most recent call last):
  File "./initial.py", line 7, in get_first_initial_bad
    return json.loads(open(file_path).read())[0]['title'][0].upper()
IndexError: list index out of range

Which index? The first [0] or the second one?

GOOD:

Traceback (most recent call last):
  File "./initial.py", line 16, in get_first_initial_good
    first_letter = title[0]
IndexError: list index out of range

Nesting statements in one line

[{"title": null}]

BAD:

Traceback (most recent call last):
  File "./initial.py", line 7, in get_first_initial_bad
    return json.loads(open(file_path).read())[0]['title'][0].upper()
TypeError: 'NoneType' object has no attribute '__getitem__'

Which is None? The JSON object or the title?

GOOD:

Traceback (most recent call last):
  File "./initial.py", line 16, in get_first_initial_good
    first_letter = title[0]
TypeError: 'NoneType' object has no attribute '__getitem__'

Nesting statements in one line

null

BAD:

Traceback (most recent call last):
  File "./initial.py", line 7, in get_first_initial_bad
    return json.loads(open(file_path).read())[0]['title'][0].upper()
TypeError: 'NoneType' object has no attribute '__getitem__'

Which is None? The JSON object or the title?

GOOD:

Traceback (most recent call last):
  File "./initial.py", line 14, in get_first_initial_good
    first_article = articles[0]
TypeError: 'NoneType' object has no attribute '__getitem__'

Nesting statements in one line

{"title": "foo"}

BAD:

Traceback (most recent call last):
  File "./initial.py", line 7, in get_first_initial_bad
    return json.loads(open(file_path).read())[0]['title'][0].upper()
KeyError: 0

Which key? The first [0] or the second one?

GOOD:

Traceback (most recent call last):
  File "./initial.py", line 14, in get_first_initial_good
    first_article = articles[0]
KeyError: 0

Nesting statements in one line

[{"title": {}}]

BAD:

Traceback (most recent call last):
  File "./initial.py", line 7, in get_first_initial_bad
    return json.loads(open(file_path).read())[0]['title'][0].upper()
KeyError: 0

Which key? The first [0] or the second one?

GOOD:

Traceback (most recent call last):
  File "./initial.py", line 16, in get_first_initial_good
    first_letter = title[0]
KeyError: 0

Nesting statements in one line

[{"title": ""}]

BAD:

Traceback (most recent call last):
  File "./initial.py", line 7, in get_first_initial_bad
    return json.loads(open(file_path).read())[0]['title'][0].upper()
IndexError: string index out of range

Which one is a string? The JSON object or the title?

GOOD:

Traceback (most recent call last):
  File "./initial.py", line 16, in get_first_initial_good
    first_letter = title[0]
IndexError: string index out of range

Specify what exception you want to catch

BAD:

def find_user(username):
    try:
        user = Users.get(username)
    except:
        user = None
    return user
>>> find_user('foo')
<User foo>
>>> find_user('bar')
None
>>> find_user(1)
None

When the user will be None? When it's not found? When the username is invalid?
It will be None even when there's a bug in Users.get(). Or somewhere deeper.

Specify what exception you want to catch

def find_user(username):
    try:
        user = Users.get(username)
    except UserLookupError:
        user = None
    return user
>>> find_user('foo')
<User foo>
>>> find_user('bar')
None
>>> find_user(1)
Traceback (most recent call last):
  File "api.py", line 7, in get
    username = username.lower()
AttributeError: 'int' object has no attribute 'lower'

We know that the user will be None when it's not found.
We know what exception is raised when a user is not found.
We know that we can catch other exceptions when calling find_user()
Avoid masking bugs.

Too much code in the `try` clause

BAD:

try:
    user = find_user(email)
    message = EmailMessage(from=user.email, subject='Foo', body='Bar')
    message.send(recipient_email)
except EmailError:
    self.log_failure(email)
return {'user_email': email, 'recipient_email': recipient_email}

Which line actually raises the EmailError?

Too much code in the `try` clause

GOOD:

try:
    user = find_user(email)
except EmailError:
    self.log_failure(email)
else:
    message = EmailMessage(from=user.email, subject='Foo', body='Bar')
    message.send(recipient_email)
return {'user_email': email, 'recipient_email': recipient_email}

Write short methods

Avoid long functions and methods.

Split them into many small ones.

Refactor functions into classes if they share the same data via arguments.

Short methods: readability

When you read a long function, at the end of it you can't remember the begining.
And you don't want to read it. (TL;DR)
If a function is long, it usually has many indentation levels. When a line is deeply indented, it has to be short. Then, there's not enough space to fit even quite short statements.
Function names can replace comments.

Short methods: testing

It's easier to unittest small functions.

Classes over functions: extending

Classes can be subclassed to extend their methods. Functions can't.

Start Writing More Classes by Armin Ronacher http://lucumr.pocoo.org/2013/2/13/moar-classes/

Short methods: bad example

class XMLParser(object):

    def validate_signature(self, xml):
        xml_sig = xml.xpath('./Signature/text()')
        timestamp = xml.xpath('./Timestamp/text()')

        # Calculate valid signature.
        sig = ''.join([timestamp, self.developer, self.app, self.certificate])
        md5sig = hashlib.md5(sig.encode()).digest()
        b64sig = base64.b64encode(md5sig).decode()

        if xml_sig != b64sig:
            raise SignatureError('XML signature is invalid.')

How to test finding the xml_sig and timestamp?
How to test calculating the valid signature?

Short methods: good example

class XMLParser(object):

    def validate_signature(self, xml):
        xml_sig = self._find_signature(xml)
        timestamp = self._find_timestamp(xml)
        valid_sig = self._calculate_signature(timestamp)
        if xml_sig != valid_sig:
            raise SignatureError('XML signature is invalid.')

    def _find_signature(self, xml):
        return xml.xpath('./Signature/text()')

    def _find_timestamp(self, xml):
        return xml.xpath('./Timestamp/text()')

    def _calculate_signature(self, timestamp):
        sig = ''.join([timestamp, self.developer, self.app, self.certificate])
        md5sig = hashlib.md5(sig.encode()).digest()
        b64sig = base64.b64encode(md5sig).decode()
        return b64sig

Comment replaced by a method name.
Each unit can be tested separately.

Methods order

__init__
Other special methods, like __repr__, __str__, __eq__, etc.
classmethods
Properties
Instance methods

Instance methods order

Public method
Helper methods in the order in which they are used

You read from top to bottom.

Methods order: bad example

class XMLParser(object):

    def _find_timestamp(self, xml):
        return xml.xpath('./Timestamp/text()')

    def _find_signature(self, xml):
        return xml.xpath('./Signature/text()')

    @classmethod
    def from_config(cls, config):
        return cls(config['developer'], config['app'], config['certificate'])

    def _calculate_signature(self, timestamp):
        sig = ''.join([timestamp, self.developer, self.app, self.certificate])
        md5sig = hashlib.md5(sig.encode()).digest()
        b64sig = base64.b64encode(md5sig).decode()
        return b64sig

    def __init__(self, developer, app, certificate):
        self.developer = developer
        self.app = app
        self.certificate = certificate

    def validate_signature(self, xml):
        xml_sig = self._find_signature(xml)
        timestamp = self._find_timestamp(xml)
        valid_sig = self._calculate_signature(timestamp)
        if xml_sig != valid_sig:
            raise SignatureError('XML signature is invalid.')

I don't know what the _find_timestamp() is used for until I read the validate_signature method.
Maybe after reading the validate_signature() I won't care about the implementation of finding the timestamp and signature.
To find out how to instantiate, I have to skip some methods which I don't care about for now.

Methods order: good example

class XMLParser(object):

    def __init__(self, developer, app, certificate):
        self.developer = developer
        self.app = app
        self.certificate = certificate

    @classmethod
    def from_config(cls, config):
        return cls(config['developer'], config['app'], config['certificate'])

    def validate_signature(self, xml):
        xml_sig = self._find_signature(xml)
        timestamp = self._find_timestamp(xml)
        valid_sig = self._calculate_signature(timestamp)
        if xml_sig != valid_sig:
            raise SignatureError('XML signature is invalid.')

    def _find_signature(self, xml):
        return xml.xpath('./Signature/text()')

    def _find_timestamp(self, xml):
        return xml.xpath('./Timestamp/text()')

    def _calculate_signature(self, timestamp):
        sig = ''.join([timestamp, self.developer, self.app, self.certificate])
        md5sig = hashlib.md5(sig.encode()).digest()
        b64sig = base64.b64encode(md5sig).decode()
        return b64sig

I can immediately see how to instantiate. Oh, and there's an alternative constructor.
Public method is what I'll probably use. For now I don't care about the helpers.
When reading the public method, I see the flow but I don't have to know the implementation of each step.
If I'm interested in how the signature is calculated, I can go to the helper method below. And I can test it without parsing the XML.

Readability counts.

Introduction

The talk title

Why

PEP8

Naming conventions

Naming conventions: class

Naming conventions: function

Naming conventions: module

Naming conventions: constant

Naming conventions: function name

Naming conventions: boolean

Naming conventions: boolean

Naming conventions: boolean

Name length

Punctuation

79 characters

Indentation of continuation lines

Indentation of a dictionary, list, etc.

Indentation of a dictionary, list, etc.

Indentation of a dictionary, list, etc.

Indentation of a dictionary, list, etc.

Indentation of a dictionary, list, etc.

Indentation of a dictionary, list, etc.

Indentation of a function call

Indentation of a function call

Indentation of a function call

Indentation of an if statement

Group your imports

Group your imports

Group your imports

Sort imports alphabetically

Use namespaces

Use namespaces: never import *!

Use namespaces: import modules

Use namespaces: import modules

Nesting statements in one line

Nesting statements in one line

Nesting statements in one line

Nesting statements in one line

Nesting statements in one line

Nesting statements in one line

Nesting statements in one line

Nesting statements in one line

Nesting statements in one line

Specify what exception you want to catch

Specify what exception you want to catch

Too much code in the try clause

Too much code in the try clause

Write short methods

Short methods: readability

Short methods: testing

Classes over functions: extending

Short methods: bad example

Short methods: good example

Methods order

Instance methods order

Methods order: bad example

Methods order: good example

Thank you

Indentation of an `if` statement

Use namespaces: never import `*`!

Too much code in the `try` clause

Too much code in the `try` clause