Привет Хабр!
Сегодня я сниму костюм аниматора и вместо развлечений расскажу вам немного за питон.
Я довольно посредственный программист, но иногда мне удаётся усыпить что-нибудь бдительность, и меня считают сеньором. И вот как-то так получилось, что я стал делать много код ревью. Просматривая файл за файлом, я вдруг увидел, что люди и проекты меняются, а вот моменты, к которым я, зануда такая, придираюсь, остаются теми же. Поэтому я решил собрать самые частые паттерны в эту сумбурную статью и надеюсь, что они помогут вам писать более чистый и эффективный питон-код.
Early quit
Это точно на первом месте, потому что везде, у всех я это вижу:
def foo(a: bool):
if a:
#
# ... 50 LOCs
#
return True
else: # if not a
return False
Когда мы только пишем/читаем код if a:
, мы запоминаем, что где-то там, в конце, нам нужно рассмотреть else
случай. Если код внутри if a
большой, то else
вообще будет "оторван" от контекста.
Мы могли бы поменять условия местами:
def foo(a: bool):
if not a:
return False
else:
#
# ... 50 LOCs
#
return True
Это легче читается, так как случай not a
мы уже рассмотрели и выкинули из головы в самом начале. Но если присмотреться, то else
вообще не нужен:
def foo(a: bool):
if not a:
return False
# в этом месте мы уже отбросили случай "not a" и забыли про него
#
# ... 50 LOCs
#
return True
В этом и вся фишка - когда пишем функцию, то стараемся как можно скорее из неё выйти при помощи отсечения каких-то плохих случаев. Эта методика прекрасна тем, что позволяет избавиться от уровней вложенности (то есть теперь никакого else
), да ещё и память программиста разгружается, потому что нет ветвления логики.
Вот синтетический пример для какого-то парсинга:
import requests
def scrape(url: str) -> dict:
response = requests.get(url)
if not response.ok: # отсекаем плохие статус-коды
return {}
# начиная с этой строчки я уверен, что у ответа хороший статус код
data = response.json()
if 'error' in data: # отсекаем случаи, когда в ответе есть ошибка
raise ValueError(f'Bad response: {data["error"]}')
# начиная с этой строчки я уверен, что нет ошибки
if 'result' not in data: # отсекаем случаи, когда ответ пуст
return {}
# начиная с этой строчки я уверен, что есть ответ
# ... parse data['result'] ...
Функция полностью "линейна", и в момент собственно парсинга данных я ничего не должен держать в голове и знаю наверняка, что всё хорошо, данные есть, ошибок нет.
Работает так же для циклов, только вместо return
будет break
или continue
.
One-line assignment
Поговорим о присваивании переменных. Часто я вижу такое:
if a:
v = 1
else:
v = 2
Тут всё довольно очевидно - зачем лишние if-else
, если можно сделать в одну строчку:
v = 1 if a else 2
А что если так
if a == 1:
v = 10
elif a == 0:
v = 0
elif a == -1:
v = -10
elif a == -2:
v = - 20
Тут вы спросите, а что будет если a not in {0, 1, -1, -2}
, и будете правы - v
будет вообще не определена. Кроме того, если я захочу позже понять, откуда появилась v
и что в ней записано, мне нужно будет посмотреть 4 случая, потому что v
определена 4 раза. И это ужасно, потому что когда переменная определена много раз, то
не факт, что вы не забыли рассмотреть ещё какой-то случай
можно опечататься и вместо
v
написатьb
, и питон это радостно съест,можно банально опечататься при копи-пасте (если вы копируете случаи и заменяете значения), и в разных случаях присвоить одно и то же значение
Какой же выход? Старайтесь определять переменные один раз. В идеале любое объявление переменной должно выглядеть так:
var = <some code>
И не более того. Я намеренно пишу "в идеале", потому что не всегда это возможно сделать, а где-то от этого страдает читаемость кода, так что нужно делать с умом.
В случае выше я бы заменил код на
v = {
1: 10,
0: 0,
-1: -10,
-2: -20,
}[a]
Тут v
определена один раз, я чётко вижу какое a
к какому v
приводит, и если a
не из допустимых значений, то всё упадёт, чему я и рад. Если вам нужно еще значения по умолчанию, то вместо [a]
используйте .get(a, default_value)
.
Definition close to usage
Ещё один способ разгрузить память программиста. Часто вижу такое:
def foo(url: str):
fields = ['a', 'b', 'c']
response = requests.get(url)
response.raise_for_status()
data = response.json()
if 'result' not in data:
raise ValueError()
for field in fields:
print(data[field])
Вообще нет ошибки, всё нормально - вот только когда я смотрю строчку for field in fields
, я уже забыл, что там в fields
, и меня ждёт увлекательное приключение в начало функции, чтобы найти там определение этой переменной. Да, в PyCharm это 2 шортката - один "jump to definition", другой "jump где я там был до этого", но было бы неплохо, чтобы вообще никуда прыгать не пришлось.
С этой проблемой и борется этот паттерн: мы определяем переменные наиболее близко к тому месту, где мы их будем использовать. Как только вы хотите создать переменную, спросите себя: "нужна ли эта переменная в следующем сниппете кода"? Если нет, то, возможно, её следует определить позднее. Таким образом при анализе кода вы сможете бросить взгляд на соседние строчки кода и понять, откуда взялись эти переменные и что в них.
В примере выше мы просто двигаем fields
именно туда, где они используются, и даже можем заинлайнить их прям в for
:
for field in ['a', 'b', 'c']:
print(data[field])
Too many indents
Питон хорош тем, что в нём есть отступы. Отступы хороши тем, что они показывают вам уровень вложенности вашей логики. Чем больше отступов, тем сложнее логика, и, соответственно, голове сложнее парсить код и держать текущий стек условий.
for x in range(10):
result = foo(x)
if result:
second = bar(result)
if second:
print(second)
else:
# <<< HERE
print('not second')
else:
print('not good')
В данном примере в строчке <<< HERE
нужно помнить, что у вас есть какой-то x
от 0 до 9, result
превращается в True
, а second
не превращается.
Нет какой-то чёткой границы, типа если у вас N
отступов, то всё плохо, а если меньше, то хорошо. Тем не менее, чем их меньше - тем лучше. Убирать отступы можно при помощи уже упомянутого "Early Quit", а также при помощи вынесения части кода в отдельную функцию.
for x in range(10):
result = foo(x)
if not result:
print('not good')
continue
second = bar(result)
print_second(second)
Частным случаем этого являются двойные, тройные и т.д. циклы, вроде
for x in range(100):
for y in range(100):
for z in range(100):
if x % 10 == 0:
if y % 10 == 0:
print('haha')
Такие штуки часто можно упростить при помощи itertools
и функций, например
from itertools import product
def print_(x: int, y: int, z: int):
if x % 10 == 0 and y % 10 == 0:
print('haha')
for x, y, z in product(range(100), range(100), range(100)):
print_(x, y, z)
Dangerous loops
Кстати о циклах! Идеальный вариант - это цикл с одним уровнем вложенности по какому-нибудь генератору:
for i in range(100):
print(i)
В реальной жизни всё несколько сложнее, но есть два простых правила, которые помогают не стрелять себе в ноги. Первое - это цикл while True
:
a = 0
while True:
a += 1
if a == 100:
break
Когда-нибудь вы поменяете a = 0
на a = 100
, и оно будет работать бесконечно. Когда-нибудь вы просто забудете написать условие выхода, или напишете такое, что не будет выполняться, или будет выполняться, но не всегда. Поверьте, бесконечное выполнение программы - это последнее, что вам хочется, а while True
- прямая дорожка к этому.
Поэтому просто что-нибудь, что кончается:
max_iterations = 100
a = 0
for i in range(max_iterations):
a += 1
if a == 100:
break
else:
raise ValueError('max_iterations reached')
Вторая категория опасных циклов - это вложенные циклы. Написать такие - раз плюнуть, но каждый вложенный цикл даёт вам ужасный прирост сложности. Цикл на 20 элементов, вложенный в цикл на 20 элементов, даёт вам 400 комбинаций - это число уже может стать боттлнеком вашей программы.
for x in range(20):
for y in range(20):
fn(x, y) # called 400 times
Решается в каждом случае индивидуально, но можно, например, исключить какие-то случаи из обработки, где это применимо:
for x, y in product(range(20), range(20)):
if 10 < x < 20 and 10 < y < 20:
fn(x, y)
Copy-paste more than twice
Копи-паста - зло. Во-первых, тяжело читать - куча одинакового кода, занимает много места. Во-вторых, слишком просто ошибиться - ребята из PVS Studio уже писали об этом
if v == 'a':
self.value_a = 1
elif v == 'b':
self.value_b = 1
elif v == 'c':
self.value_c = 1
Обычно копи-паста легко схлопывается в один сниппет кода, либо при помощи каких-то маппингов через словари, либо при помощи getattr / setattr
, либо выделением копи-пастного кода в отдельную функцию и вызова её с параметрами. Иными словами, весь копи-пастный код проводится к общему виду и потом параметризуется.
setattr(self, f'value_{v}', 1)
Interconnected lines of code
Тут всё просто - бывают строчки кода, которые связаны, то есть когда вы меняете одну из них, должна поменяться и другая. Понятно, что однажды вы поменяете одну и забудете поменять другую. Именно поэтому от них нужно избавляться, как правило ссылаясь из одной строчки кода на другую:
rows = [
{'col 1': 'value 1', 'col 2': 'value 2'},
{'col 1': 'value 3', 'col 2': 'value 4'},
]
writer = csv.DictWriter(..., fieldnames=['col 1', 'col 2'])
Теперь если вы решите добавить ещё одну колонку в rows
, или переименовать существующую, то вам также нужно будет изменить fieldnames
. Вместо этого мы заставим fieldnames
"ссылаться" на нужные значения:
rows = [
{'col 1': 'value 1', 'col 2': 'value 2'},
{'col 1': 'value 3', 'col 2': 'value 4'},
]
writer = csv.DictWriter(..., fieldnames=rows[0].keys())
Type hints
Тут всё просто: всегда используйте type hints. Когда вы пишете код, вам весело и приятно, но когда ваш код читают (а может это будете и вы сами через год), очень тяжело понять, что это за аргументы у функции и какого они типа. В этом плане type hints хотя бы немного помогут.
def foo(processor):
# wtf is processor? what is returned?
...
def foo(processor: Processor) -> str:
# okay, now I can jump to definition of Processor and see what it is
...
Я пишу "хотя бы", потому что type hint типа Tuple[int, str, datetime]
никак не говорит вам, что это на самом деле возвращается (object_id, name, creation_datetime)
. Тут помогли бы namedtuple
, но использовать их для возвращаемых значений кажется оверинжинирингом.
Quick "in" check
Тут всё просто до безобразия: хотите проверить вхождение чего-то во что-то? Юзайте set
(множество), ведь element in set
выполняется за константное время. Как только вы делаете element in list
, вы вызываете демона поэлементного сравнения, который рано или поздно укусит вас за зад при большом количестве элементов в списке.
Не бро:
items = [instance1, instance2, instance3]
if instance4 in items:
print('yes')
Бро:
items = {instance1.id, instance2.id, instance3.id}
if instance4.id in items:
print('yes')
Bulk
Вот это прям всем, всегда. Итак: всегда пишите код сразу для миллиона объектов, даже если у вас их сейчас два.
Загружаете файлы в облачное хранилище? Делайте в несколько потоков, как будто вам надо загрузить миллион файлов.
Пишете SQL запрос? Делайте джойны, как будто у вас миллион записей в каждой таблице.
Пишете view для Джанго? Пишите его так, как будто его будут вызывать миллионы пользователей.
Пишете код для загрузки данных? Пишите так, как будто будете загружать миллионы строк.
На самом деле это просто, стоит только привыкнуть. Для SQL это какие-нибудь JOIN
и INDEX
, для Django это select_related
, only
, values_list
, update
и bulk_create
, где-то ещё это ThreadPoolExecutor
. Это сложнее, чем работать по-объектово, где-то придётся дробить входне данные на чанки, где-то отправлять параллельно, но зато ваш код будет работать и для одного объекта, и для миллиона. Может их и не станет миллион, но однажды их станет не два, а тысяча, а ваш код будет работать всё так же быстро.
Concurrency safety
Однажды ваш код запустят не в вашем любимом терминале, а в потоках. Или в процессах. На разных машинах. Что случится, когда вы будете обрабатывать одновременно одни и те же данные? Делать одни и те же запросы к внешним API? Обращаться к одним и тем же файлам на диске?
Возможно, ничего. Возможно, ничего хорошего.
Представьте, что после каждой строчки кода выполнение вашей программы может быть прервано, и управление перейдёт другой программе - или точно такой же. Теперь пишите код с учётом этого.
Подобные мысли приводят к появлению таких вещей, как, например, threading.Lock
, @transaction.atomic
, SELECT FOR UPDATE
и иже с ними.
Asserts everywhere
И последнее - немного про assert
. Я считаю, что это замечательная вещь, потому что лучше "либо хорошо, либо никак", а assert
как раз про это - у вас либо всё хорошо, либо всё падает, а третьего не дано.
Ожидаете определённые данные с внешнего сервиса? Ставьте ассерт:
result = get(service_url).json()
assert result in {'ok', 'fine', 'good'}
Ожилаете, что есть хотя бы один Item
? Ставьте ассерт:
items = Item.objects.all()
assert len(items)
print(items[0])
Написали код, который сами не понимаете? Ставьте ассерт:
result = foo(bar(baz(x)))
assert result is not None
Общее правило: если вы что-то ожидаете или в чём-то не уверены, то ставьте assert
.
Стоит, однако, помнить, что assert
могут быть отключены при помощи заклинания -O
, но если вы встретите тех, кто так делает, то передайте им от меня пламенный привет и немного новичка.
К чему всё это
Как видите, цель этих правил - улучшить читаемость кода, разгрузить память программиста и уменьшить число ошибок. Тут нет какой-то рокет саенс, все пункты достаточно простые, но если вы будете их придеживаться, то мне будет проще в случае, если ваш код когда-нибудь попадёт мне на ревью >:)
eumorozov
Первые пункты хороши, особенно для новичков.
С последними правилами не совсем согласен.
Quick "in" check
Именно в таком варианте использования разница пренебрежимо мала.
Если же речь идёт не о нескольких предопределенных константах, то это уже другой случай, и относится скорее к пониманию разработчиком структур данных, что спрашивают на каждом интервью. Кроме того, по большому счёту, в реальной разработке даже на списках длиной в тысячи элементов разница будет незаметна, т.к. время будет потрачено на выполнение сотни-другой запросов к SQL серверу, скажем, каждый из которых только на сетевой раунд-трип потратит больше времени.
Bulk
Преждевременная оптимизация. По своему большому опыту могу сказать, что одноразовые скрипты чаще всего и остаются одноразовыми. А попытки предугадать, где же надо оптимизировать, чтобы выдержало большую нагрузку, чаще попадают мимо. Можно потратить кучу времени на оптимизацию какого-то куска, который выглядит подозрительно, чтобы после выкатки в прод и наплыва пользователей убедиться, что порвалось совсем в другом месте, которое почти невозможно было предвидеть заранее.
Уж во всяких вспомогательных скриптах загрузки данных преждевременная оптимизация точно зло и напрасно потраченное время. Даже если он выполняется 4 часа вместо 15 минут, быстрее и дешевле просто запустить его в отдельной консоли, и забыть. Или использовать что-то типа:
Есть даже готовое решение для fish: done
Concurrency safety
Преждевременная оптимизация и бесполезная к тому же. В коде на Python почти никогда не используются потоки. Да и смысл тратить кучу усилий для того, чтобы обработать варианты ситуаций, которые с вероятностью 99.99% не случатся. Лучше это время потратить хотя бы на поиск уязвимостей в своём коде.
Asserts everywhere
Сомнительное правило с сомнительными примерами. Например, вместо
проще написать
get
и так поднимет исключение, если элементов нет или более одного.sshikov
>разница пренебрежимо мала.
Это вы про разницу между 163 и 119? Это ведь почти 30%, особенно с учетом того, что решение-то тривиальное, и нам ничего не стоит. Я такие случаи регулярно на review ловил в реальных приложениях, и разница в конечном счете обычно была очень даже заметна.
kesn Автор
Quick "in" check
Ничего не стоит при написании, забесплатно даёт O(1) вместо O(n), не делает лишние сравнения, если есть дубликаты в списке. Я за этот вариант.
Bulk
В скриптах "для себя" - ок. В коде для клиента я лучше пострадаю, но напишу сразу с запасом, чтобы потом в один прекрасный день где-то через год мне не написали, что поток запросов вырос и всё упало.
Concurrency safety
Значит я особенный, потому что я использую постоянно. Плюс я говорил про процессы в том числе.
0.01% обязательно случится.
Asserts everywhere
Согласен, пример неудачный
danilovmy
Эээ ребят, а это ничего что это не работает?
get() вернет объект у которого, скорее всего, нет [0] или упадет DoesNotExist/MultipleObjectsReturned.
Вероятно, подразумевалось Item.objects.all()[:1], но тогда это просто Item.objects.last() или Item.objects.first() в зависимости от задачи.
eumorozov
Тяжело отвечать, когда есть возможность делать это только раз в день. Экономишь ответы «на всякий случай».
На большинство возражений просто скажу: в моей практике почти никогда не случается то, что пытаешься запланировать заранее.
В том смысле, можно потратить в два раз больше времени на то, чтобы сделать код threadsafe, ожидая, что именно этот код понадобится параллелить в будущем, но это никогда не происходит. Либо потому что приходится переписывать эту часть целиком из-за изменившихся требований. Либо выясняется, что на эту часть кода нагрузка настолько минимальна, что хватает однопоточного выполнения. Либо ещё что-то.
Можно было бы сказать, что это лично мой опыт, но я также постоянно вижу это со стороны. В том числе, как насчет будущего ошибаются гораздо более умные и опытные разработчики.
Поэтому я почти не стараюсь угадать будущее (за исключением очевидных вещей и интуиций, которые у каждого накапливаются с опытом, типа проблемы N+1 запросов в ORM). Пишу просто наиболее простой код, подходящий для решения проблемы. Если он перестанет устраивать в будущем, то намного проще будет переписать/адаптировать его с учётом новых условий, чем сразу потратить 20x времени на попытки предугадать будущее, придумать архитектуру, подходящую для всех возможных вариантов нагрузки и окружений, и затратить кучу времени на её реализацию.
Что касается микрооптимизаций типа
val in set
,val in list
, то в большинстве проектов 80% процентов времени тратится в 20% кода. Можно потратить часы на микрооптимизации, снимающие 30 нс в каком-то месте, чтобы потом не заметить этого вообще из-за сетевого джиттера при взаимодействии кода с сервером БД, redis, memcache, и т.д. Такие оптимизации есть смысл делать в самую последнюю очередь, или в самых высоконагруженных участках, предварительно обязательно проверив профайлером, что они действительно вызываются очень часто. Чтобы не потратить время на микрооптимизацию функции (которую ещё обязательно надо протестировать, т.к. ошибки могут возникнуть при любом изменении, а это дополнительное время), которая вызывается один раз за всё время работы приложения.erdbeeranke
Quick "in" check
а создание множества разве бесплатно? тот же O(n)
saintbyte
Пожалуйста с сравните аналогичными структурами numpy
tropico
Например в Cisco десятки огромнейших бэкенд сервисов были написаны на чистом трединге и мультипроцессинге и в продакшене ранились годами в такой части бизнеса, которая приносит миллиарды.
YuriM1983
Спикайте уж сразу на инглише, плиз.
tropico
Эти слова намного легче говорить и понимать, ежели душные формальные обозначения, которые слышишь раз в пять лет в каком-то докладе, от которого засыпаешь через 2 минуты.
YuriM1983
Да успокойтесь вы :) Не обязательно отвечать на все мои PS, как меня «сильно задело».
Сразу вспоминается одно видео...