Чистый код
Недавно попал на обсуждаение критики книги "Чистый код". Книга известная, у меня даже в каких-то списках "на прочитать" довольно долго висела, а тут оказывается что уже и читать ничего не надо.
На своих проектах я использую RuboCop, это линтер-форматтер-статический анализатор кода для Ruby. Вот на днях писал метод для класса, и после прогона рубокоп ругнулся на Method Length и Average Branch Condition Size метрики. Типа метод сильно длинный да еще и сложный.
Вот метод, суть такая—есть чек, за чек даём приз, не больше 50 на пользователя, общее количество призов ограничено (они заранее созданы):
def create_prize
return unless state == 'APPROVED'
if prize
Rails.logger.info("Prize for receipt #{id} and user #{user.id} was already created")
return
end
user.with_lock do
# count prizes and check per-user limits
total_prizes = Prize.where(user_id: user.id).count
if total_prizes >= 50
Rails.logger.info("User #{user.id} already received 50 prizes")
return
end
# find available prize
new_prize = Prize.lock.where(user_id: nil).first
if new_prize.nil?
Rails.logger.info("No prizes left")
return
end
new_prize.with_lock do
new_prize.update(user_id: user.id, receipt_id: id)
rescue ActiveRecord::RecordNotUnique
Rails.logger.info("Prize for receipt #{id} and user #{user.id} was already created")
end
end
end
Все просто, открываем транзакцию и едем по условиям. Но рубокопу и (очевидно дяде Бобу—автору чистого кода) такой код не нравится. Слишком много строк. Слишком много условий.
Вот я смотрю на этот код и не понимаю как его сократить так чтобы не потерять читабельность. Нет, можно вынести какие-то блоки в отдельные методы, но от этого потеряется целостность. У меня перед глазами готовый алгоритм, конкретные условия. Как его уменьшать? Раньше я заморачивался по размеру функций (не больше 9 строк!) и просто делал extract method, но сейчас постарел и просто не понимаю зачем это делать.
Можно выковырять функцию типа limit_reached?
где проверить условия по лимитам и еще одну функцию create_prize
(тогда придется базовый метод переменовать в grant_prize
и оставить там одну строку create_prize unless limit_reached?
. Будет отличный метод из одной строки (чистый код!), но мне кажется что от этого потеряется читабельность, возрастёт когнитивная нагрузка на переходы между методами и понимание что происходит.
Пока нет нужды переиспользовать эти блоки нет смысла городить абстракции.
Сокращать условия в односторчники, например схлопнуть первое условие в return if state != 'APPROVED' || prize.present?
, убирать дебаг информацию мне кажется тоже усложнением а не упрощением. Scala код очень компактен но я не могу его читать из-за своей глупости и предпочитаю когда всё маскимально разжёвано.
Вот и получается что рубокоп ругается на все методы которые что-то делают и мне приходится его вручную попускать. Начинаю подумывать, а оно мне вообще надо—проверка на длину методов? Если сильно длинный то и так видно, спасибо, кэп, сам поправлю.
А как у вас с длинными методами? Что делать с этим? Фигачить еще или рефакторить в однострочник? Переписать на го? Переписать на раст? Перестать программировать?