Прежде чем перейти к статье, хочу вам представить, экономическую онлайн игру Brave Knights, в которой вы можете играть и зарабатывать. Регистируйтесь, играйте и зарабатывайте!
Как говорил Парацельс, всё есть яд и всё есть лекарство — зависит от дозы. Например, не очень опытный разработчик обычно может из простого класса сделать три с использованием метаклассов и прочих, не так часто используемых в реальной жизни конструкций языка. И объяснит это принципом единственной ответственности так, как он его понимает — надо делить код на функции и методы, которые выполняют одну узкую и специфическую работу.
Да, самый очевидный способ упростить жизнь разработчику — это поделить большой кусок кода на несколько небольших, но делающих одну конкретную работу и делающих её хорошо. Это UNIX-way в разработке. Наделять классы, функции, модули, пакеты, сервисы и т. д. единственной ответственностью кажется естественным и понятным. Более того, кажется, что этого достаточно для того, чтобы хорошо декомпозировать код на меньшие части.
Но принцип на то и принцип, чтобы не быть непреложным законом. Нужно чётко понимать границы его применимости и, более того, наивное восприятие принципа единственной ответственности.
Для иллюстрации попробуем взять относительно несложный кусок кода и по-разному его декомпозировать. Для простоты будем считать, что у нас есть единственный инструмент декомпозиции — выделение в отдельную функцию, а фреймворк отсутствует. И постараемся понять, какое разбиение лучше или хуже, какое соответствует принципу единственной ответственности, а какое нет.
Например, у нас есть код контроллера из мифического приложения «Блог». В контроллере стандартная последовательность действий:
берём из сессии пользователя;
получаем данные поста;
проверяем доступы;
проверяем данные из формы;
обновляем данные в базе.
@bp.route('/<int:post_id>/update', methods=('POST'))
def update_post(post_id):
db = get_db()
user_id = session.get('user_id')
# Получаем данные о пользователе из БД
user = db.execute(
'SELECT * FROM user WHERE id = ?', (user_id,)
).fetchone()
# Получаем данные о посте из БД
post = db.execute(
'SELECT p.id, title, body, created, author_id, username'
' FROM post p'
' WHERE p.id = ?',
(post_id,)
).fetchone()
# Если пост не найден, кидаем 404-ю
if post is None:
abort(404, "Post id {0} doesn't exist.".format(id))
# Проверяем, что пост принадлежит автору
if post['author_id'] != user['id']:
abort(403)
# Валидируем заголовок из формы в запросе
title = request.form['title']
if not title:
error = 'Title is required.'
# Валидируем тело поста
body = request.form['body']
if not body:
error = 'Body is required'
# Если всё без ошибок, обновляем данные в базе и редиректим на пост
if error is not None:
flash(error)
else:
db.execute(
'UPDATE post SET title = ?, body = ? WHERE id = ?',
(title, body, id)
)
db.commit()
return redirect(url_for('blog.index'))
# redirect to update
return render_template('blog/update.html', post=post)
Очевидно, что в этом коде слишком многое делается. Читать такой код сложно. И его надо разделить на части.
Попробуем раздекомпозировать кусок кода, который выбирает из базы пост по id и проверяет права доступа:
# Получаем данные о пользователе из БД
user = db.execute(
'SELECT * FROM user WHERE id = ?', (user_id,)
).fetchone()
# Получаем данные о посте из БД
post = db.execute(
'SELECT p.id, title, body, created, author_id, username'
' FROM post p'
' WHERE p.id = ?',
(post_id,)
).fetchone()
# Если пост не найден, кидаем 404-ю
if post is None:
abort(404, "Post id {0} doesn't exist.".format(id))
# Проверяем, что пост принадлежит автору
if post['author_id'] != user['id']:
abort(403)
Что с этим куском можно сделать? Этот кусок кода можно положить в отдельную функцию целиком. А потом, например, отрефакторив, сделать вместо двух запросов в БД один.
def get_post_for_user_id(db, post_id, user_id):
# Получаем данные о посте из БД
post = db.execute(
'SELECT p.id, title, body, created, author_id, username'
' FROM post p'
' WHERE p.id = ? AND p.author_id = ?' ,
(post_id, user_id)
).fetchone()
return post*Немного поменялась семантика, мы уже не отдаём 403 при отсутствии прав, но:
Это может быть даже лучше и безопаснее.
Закроем на это глаза.
Или, наоборот, можем сделать разделение более гранулярным, выделить несколько функций:
def get_user_by_id(db, user_id):
user = db.execute(
'SELECT * FROM user WHERE id = ?', (user_id,)
).fetchone()
return user
def get_post_by_id(db, user_id):
post = db.execute(
'SELECT p.id, title, body, created, author_id, username'
' FROM post p'
' WHERE p.id = ?' ,
(post_id,)
).fetchone()
def can_user_edit_post(user, post):
return user['id'] == post['author_id']
Какой из этих двух вариантов лучше и правильнее?
1-й:
post = get_post_for_user_id(db, post_id, user_id)
if not post:
abort(404)
2-й:
user = get_user_by_id(db, user_id)
post = get_post_by_id(db, post_id)
if not can_user_edit_post(db, user, post):
abort(404)И первый, и второй варианты снижают когнитивную нагрузку на восприятие кода, его становится проще читать. И каких-то причин считать, что один из вариантов с точки зрения читаемости лучше — нет. Семантика «возьми мне пост из базы по id, который принадлежит этому пользователю» — понятная, конкретная задача. Но и «возьми объект», «возьми пользователя», «проверь, может ли пользователь изменять объект» — тоже набор конкретных и понятных задач.
Опять-таки с точки зрения читаемости все три варианта — ок. Нет ни одной веской причины предпочесть хотя бы один из них. Мы можем почти бесконечно сужать, расширять, обобщать, конкретизировать, модифицировать область ответственности любого куска кода. Вариантов разбиения сложного кода, которые снижают когнитивную нагрузку восприятия, — бесконечно много. Но нужно вспомнить, что работа программиста — это изменение кода, а не просто чтение. И то, что код красиво выглядит и хорошо читается, не значит, что в дальнейшем его будет проще править.
Хороший или плохой код определяется тем, легко ли будет в дальнейшем исправить его. Типичный аргумент, который приводится, когда разработчики решают, нужно ли делать так или иначе, звучит как «а что, если?»
«А что, если тебе понадобится использовать вместо Posgresql Mysql или Oracle?»
«А что, если у тебя в списке будет не 5 записей, а 30, 3000?»
«А что, если у тебя поменяется реализация алгоритма?» и т. д.
В общем суть аргумента проста — насколько твой код готов к будущим изменениям? Потому что именно это определяет качество кода — насколько легко его будет править и читать (для того, чтобы потом исправлять).
Часто молодые и горячие программисты в угоду универсальности приносят всё, придумывают невероятные крайние ситуации и под них пишут код, часто готовятся к ситуациям, которые никогда не произойдут. Собственно, и сам Боб Мартин переформулировал принцип единственной ответственности в терминах будущих изменений:
Если посмотреть с этой точки зрения на функциональное разделение кода в нашем примере, ошибки при распределении зон ответственности в случае таких маленьких функций стоят не очень много, да вообще ничего, по сути, не стоят. В конце концов, как бы ни был написан этот код, его слишком мало, чтобы вызвать настоящие проблемы при правке или чтении. Но в случае ошибок при наследовании одного класса от другого или разделении кода на сервисы в рамках сервисной и микросервисной архитектуры, исправить и жить намного тяжелее.
Само по себе наделение какой-либо ответственностью не бывает правильным или неправильным. Нет единственно верного разделения или выделения ответственностей для классов, функций, методов, модулей, пакетов и т. д. Выделяя код в отдельные абстракции кода — в функции, классы, объекты, модули, пакеты, сервисы и т. д., мы проводим границы, которые либо будут способствовать дальнейшим правкам, изменениям и простоте чтения кода, либо нет. Разделение — это плохо, если дальнейшие изменения становятся сложнее, а не проще.
