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
Practical Clean Code - CommitConf2018
- 2,121