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

References

thanks! :)

Practical Clean Code - CommitConf2018

By Israel Saeta Pérez

Made with Slides.com

Practical Clean Code - CommitConf2018

  • 2,141

More from Israel Saeta Pérez