Coder Social home page Coder Social logo

doebus's Introduction

Кодекс Питонус Доёбус

Привет! Меня зовут Всеволод, и я очень люблю доёбываться до мелочей. К счастью, я работаю в компании, где мне за это ещё и платят. Чтобы продуктивнее доёбываться, я веду список всех ошибок, которые часто встречаю на ревью у новеньких в нашей компании. Ниже не закон, а мой опыт

Самые новые ошибки вверху, чтоб легче было возвращаться к этому документу и читать его до того момента, пока не поймёшь что ниже уже читал. Поехали…

Если переменная имеет тип datetime, то должна заканчиваться на _at. Если переменная имеет тип boolean, то должна начинаться на is_, are_, was_, has_ и так далее. Причина в том, что если назвать переменную deleted, то невозможно понять: это дата удаления или флажок что оно удалено? Если переменная хранит в себе факт нужды совершить действие, например Announcement(notify_subscribers=True), то переменная должна иметь префикс do_ и называться do_notify_subscribers. Причина в том, что в 9 случаев из 10 со временем появляется метод notify_subscribers и его путают с переменной

❌ updated, updated, update 
⭕️ updated_at, is_updated, do_update 

«Юзеры на удаление» переводится не как users_for_delete, а как users_to_delete. Если последнее слово глагол, то используем to, если существительное, то for

❌ posts_for_add, users_to_deletion
⭕️ posts_to_add, users_for_deletion

Паттерн Arrange Act Assert означает что тест поделён на три части, разделённые пустыми строками — подготовка (пара строк), действие (одна строка) и асерты (сколько угодно). Таково самое лучшее деление. Часть с подготовкой может быть опущена если она выполняется на уровне фикстур. Действия типа refresh_from_db являются частью асерта

def test_publish_post(factory):
    post = factory.post()
    post_publisher = PostPublisher(post)
    post = post_publisher()
    assert post.is_published is True
⭕️ 
def test_publish_post(factory):
    post = factory.post()
    post_publisher = PostPublisher(post)
    
    post = post_publisher()
    
    assert post.is_published is True
⭕️ 
def test_send_mail(mail):
    mail.send()
    
    assert mail.is_sent is True
⭕️ 
def test_unpublish_post(post):
    post.unpublish()
    
    post.refresh_from_db()
    assert post.is_published is False

Исторически бизнес-логика хранилась в моделях, но опыт показывает что лучше её хранить в сервисах, потому что сервисы легче тестировать и поддерживать. Сервис — это небольшой класс, который наследуется от BaseService и выполняет какой-то кусочек бизнес-логики. Типичные сервисы — PostCreator, UserImporter, MembershipUpdater и так далее. Самое главное место в сервисе — метод act, перекрываем его и пишем туда код сервиса. Если нужна валидация, то расширяем метод validate. В 95% случаев сервис должен возвращать какой-то объект или объекты и обычно это тот объект или объекты, над которыми выполняется действие. Много маленьких сервисов — это хорошо

class Post(DefaultModel):
    ...
    
    def publish(self):
        self.is_published = True
        self.published_at = now()
        self.save()
⭕️
class PostPublisher(BaseService):
    def __init__(self, post):
        self.post = post
    
    def act(self):
        self.post.is_published = True
        self.post.published_at = now()
        self.post.save()

Часто надо написать в тестах следующее: test_something_when_something. В этих сценариях лучше писать не when, а if, потому что if короче и естественнее для программиста, мы часто видим ветвление if и для нас когнитивная нагрузка от этого слова куда ниже чем от слова when

❌ test_membership_when_anon
⭕️ test_membership_if_anon

Нельзя называть переменные именами got, result, x и так далее. Причины две. Первая: банально непонятно что внутри, это создаёт лишнюю когнитивную нагрузку на читающего. Вторая: это соблазняет не понимать собственный код. Когда разраб пишет код, он должен железно понимать его, он должен знать что внутри каждой переменной. Названия got или result соблазняют забивать на то, что внутри, а это ведёт к проблемам. Исключения у правила есть, к примеру в тестах АПИ ответ всегда называется got, потому что печатать response слишком долго, а кроме него ничего в АПИ никогда не бывает. Но если в got или result может попасть всё что угодно, то их писать нельзя, надо писать чётко что попадёт внутрь

❌ got = membership_creator()
⭕️ membership = membership_creator()

В тестах часто бывает нужно немного отклониться от стандартной фикстурной версии модели, в результате появляется соблазн просто взять и впатчить туда нужное. От этого код становится хрупким, потому что джанговские модели устроены таким образом, что если ты впатчишь им атрибут, который на самом деле не является полем, то ошибки не случится. Если у модели есть поле author, а я опечатаюсь и в фикстуре сделаю post.athor = user, то ошибки не случится, это может породить неуловимые проблемы. Но что ещё хуже: впатчивая некий кусок данных в инстанс модели, мы тем самым относимся к модели исключительно как к данным, а не как к сущности бизнес-логики. Если при создании инстанса Post в случае указания поля author должен выполниться некий сторонний сервис, то при post.author = user он не выполнится. В результате тесты начнут тестировать искусственную функциональность, которая пропускает часть бизнес-логики. В фикстурах инстанс модели никогда не должен меняться с момента создания до момента экшена, он всегда должен доходить до экшена ровно таким, каким он был создан

def test_post_of_user(post, user):
    post.author = user
    post.save()

    assert user.authored_posts.first() == post@pytest.fixture
def post_of_user(user):
    post = factory.post()
    post.author = user
    post.save()
    return post


⭕️
@pytest.fixture
def post_of_user(user):
    return factory.post(author=user)

Если бизнес-логика подразумевает, что публичный пост — это пост опубликованный, политкорректный и так далее, то вся эта логика должна проверяться в одном хорошо известном месте, обычно это метод менеджера. Нельзя распылять эту логику по другим приложениям, потому что тогда мы обязываем себя не забыть про все такие места, когда в понятие «публичного поста» добавится ещё какое-нибудь условие

Распылять по приложениям:

# В кверисете для вывода постов на главной в апе posts
public_posts = Post.objects.filter(is_published=True, is_restricted=False)

# В кверисете для вывода постов этого автора в апе users
authored_posts = Post.objects.filter(is_published=True, is_restricted=False, author=user)

# В кверисете для вывода постов в апе sitemaps:
sitemap_posts = Post.objects.filter(is_published=True, is_restricted=False, published_at__gt=week_ago)
⭕️ Хранить централизованно:

# Кверисет-менеджер модели Post
class PostQuerySet(DefaultQuerySet):
    def public(self):
        return self.filter(is_published=True, is_restricted=False)


# В кверисете для вывода постов на главной в апе posts
public_posts = Post.objects.public()

# В кверисете для вывода постов этого автора в апе users
authored_posts = Post.objects.public().filter(author=user)

# В кверисете для вывода в сайтмапы:
sitemap_posts = Post.objects.public().filter(published_at__gt=week_ago)

Если сервис называется PostCreator, то когда мы инстанцируем его, мы должны назвать его post_creator, а не просто creator. Во-первых, без этого поиск post_creator по проекту пропустит это место. Во-вторых, когда человек читает код, увидев creator ему придётся гадать о том, что это за криэйтор: user_creator, post_creator, tag_creator и так далее. Не надо создавать людям лишнюю когнитивную нагрузку

❌ creator = PostCreator()
⭕️ post_creator = PostCreator()

Если в названии теста используется глагол в третьем лице, то не надо ставить s на конце. Это нужно, чтобы по коду было легче найти все вхождения действия из разряда create_post. Если в поиске вбить только create_, то это выдаст кучу лишнего типа create_user, create_tag и так далее, а если вбить create_post, то поиск не найдёт тесты с названиями типа creates_post. Поэтому s не нужна

❌ test_user_creates_post
⭕️ test_user_create_post

В названии переменных не нужно указывать типы, потому что это накладывает необходимость не забыть поменять название переменной, если внутри поменяется тип данных, а это происходит повсеместно, чаще всего list превращается в dict. Другая причина — часто туда может попасть None

❌ user_id_list
⭕️ user_ids

На одной строке должен быть минимум действий, иначе увеличивается когнитивная нагрузка на читающего и появляются ошибки от невнимательности к концу строки. Люди читают слева направо и фокус внимания падает по мере движения взгляда вправо

❌ Post.objects.published().filter(published__gte=recently_period).values_list('id', flat=True)
⭕️ Post.objects \
    .published() \
    .filter(published__gte=recently_period) \
    .values_list('id', flat=True)

Магические числа — это числа, которые означают непонятно что. Ниже пример, где невозможно понять: семь чего? Дней, минут, часов?

❌ Post.objects.published_recently(7)
⭕️ Post.objects.published_recently(days=7)

Noun adjunct: если мы используем существительное в роли своего рода определения для другого существительного, то оно должно стоять в единственном числе. Если у нас есть массив cats и массив names, а потом мы их склеиваем, то результат должен называться cat_names, а не cats_names. Мы же например говорим backend developers, а не backends delelopers. Есть исключения типа sports companies, но в 99% случаев речь не об исключениях

❌ views_count
⭕️ view_count
❌ users_posts
⭕️ user_posts

Если в переменной есть in или from, то это намёк на то, что её нужно развернуть задом-наперёд. В английском почти что угодно может выполнять роль определния, поэтому более естественно писать часть после in в начале, а само in выкидывать

❌ posts_in_feed
⭕️ feed_posts
❌ user_from_popularity_widget
⭕️ popularity_widget_user

Если в названии теста есть слово and, то он скорее всего тестирует больше чем одну единицу кода, его надо или разделить на два теста или слепить две единицы кода в единицу более высокого уровня

❌ test_first_name_and_last_name
⭕️ test_first_name, test_last_name
⭕️ test_name_fields

doebus's People

Contributors

42juododranoel avatar

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.