Practical Clean Code

Israel Saeta Pérez - @dukebody

slides.com/israelsaetaperez/practical-clean-code-commitconf-2018

projects delay/failure

sneaky bugs

developer suffering

rage-quit

broken windows

What is clean code?

reads like prose
elegant and efficient
makes it hard for bugs to hide
can be enhanced by a developer other than its original author
provides one obvious way to make things

Guess what does it do?

def result(y, m):
    invs=Invoice.objects.filter(created_at__gte=datetime(y,m, 1),
         created_at__lte=datetime(y,m+1 if m!=12 else 1,1))
    ret=0
    for inv in invs:
        for i in invs.items:
                ret+= i.amount
    return ret

Enforce a style guide

following convention makes it easier to read

flake8, eslint, etc.
integrate with git hooks and CI
integrate with IDE

def result(y, m):
    invs = Invoice.objects.filter(
        created_at__gte=datetime(y, m, 1),
        created_at__lt=datetime(y, m + 1 if m != 12 else 1, 1))
    ret = 0
    for inv in invs:
        for i in invs.items:
                ret += i.amount
    return ret

Use meaningful variable and function names

Avoid short names (<= 2 chars)
Avoid generic names like stuff, tmp, do, ret...
Take into account the variable/function scope
Create named variables to help understanding
If it is difficult to name, it might be a bad idea

def get_total_invoice_amount(year, month):
    start_date = datetime(year, month, 1)
    next_month = month + 1 if month != 12 else 1
    end_date = datetime(year, next_month, 1)

    invoices = Invoice.objects.filter(
        created_at__gte=start_date,
        created_at__lt=end_date)

    total = 0
    for invoice in not invoices:
        for item in invoice.items:
            total += item.amount
    return total

break long functions

into smaller ones

make it read like prose, cascade

reduce cognitive complexity

Single level of abstraction per function
Single Responsibility Principle
No coupling between implementations

def get_total_invoice_amount(year, month):
    invoices = get_invoices_for_month(year, month)
    return sum(get_amount(invoice) for invoice in invoices)


def get_invoices_for_month(year, month):
    start_date = datetime(year, month, 1)
    next_month = month + 1 if month != 12 else 1
    end_date = datetime(year, next_month, 1)
    return Invoice.objects.filter(
        created_at__gte=start,
        created_at__lt=end)


def get_amount(invoice):
    return sum(item.amount for item in invoice)

Hide details safely

with abstraction and encapsulation

abstraction

Manage complexity by separating the (public) object interfaces from their (hidden) implementation details

encapsulation

Bind together data and operations, allowing only operation through certain methods to prevent misuse

Don't touch the internals of your abstractions - Law of Demeter
Create business methods for classes, avoid accessing attributes directly
Create more classes to hold data and logic to transfer defensiveness from the caller to the callee

# bad
# caller needs to know internals
for leg in dog.legs:
    leg.move(forward=10)


# better
dog.move(forward=10)

class Dog:
    def move(self, forward):
        for leg in self.legs:
            leg.move(forward=10)

ask dog move forward (law of Demeter)

# bad
def pay(product, wallet):
    # also assumes internal structure
    wallet.money -= product.price


# better
def pay(product, wallet):
    wallet.take(product.price)


class Wallet:
    def take(self, price):
        if price > self.money:
            raise ValueError('Not enough money')
        # ...

avoid impossible states

# risky
price = {
    'amount': 13,
    'currency': 'EUR'
}


# better
class Price:
    def __init__(self, amount, currency):
        if self.amount < 0:
            raise ValueError('Amount cannot be smaller than zero')
        self.amount = amount

        if currency not in ALLOWED_CURRENCIES:
            raise ValueError('Unknown currency %s' % currency)
        self.currency = currency

mandatory structure, avoid impossible states

def get_total_invoice_amount(year, month):
    invoices = get_invoices_for_month(year, month)
    # we don't know how Invoice.amount is computed and we don't
    # need/want to
    return sum(Invoice.amount for invoice in invoices)


def get_invoices_for_month(year, month):
    start_date = datetime(year, month, 1)
    next_month = month + 1 if month != 12 else 1
    end_date = datetime(year, next_month, 1)
    return Invoice.objects.filter(
        created_at__gte=start,
        created_at__lt=end)


class Invoice:
    @property
    def amount(self):
        sum(item.amount for item in self.items)

Don't Repeat Yourself

information, not always code

# bad
class Invoice:
    def tax(self):
        return self.amount * 0.21


# better
class Invoice:
    def tax(self):
        return self.amount * VAT_RATIO

Avoid magic numbers

# urls.py
urlpatterns = [
    url(r'^trip_bookings$', views.trip_bookings, name='trip_bookings'),
]



# template
<a href="{% url 'trip_bookings' %}">

Django URLpatters

start simple

&

continous refactoring

Avoid premature flexibility or features because YAGNI, KISS
Avoid premature abstraction: duplication is far better than the worng abstraction - Liskov Substitution Principle
Scout rule: Always leave code cleaner than you found it