Один из самых распространённых принципов, часто упоминаемых в отзывах к пул-реквестам — это Don’t Repeat Yourself («не повторяйся»). Изначальные предпосылки для использования принципа DRY были вполне разумными.
Однако со временем, как бывало и со многим другим в разработке ПО, суть и смысл его существования, на мой взгляд, был утерян. Вместо того, чтобы применять его по необходимости, его засовывают везде, где возникает хотя бы намёк на дублирование, и я считаю, что в долговременной перспективе это приводит к ухудшению кода.
В этой статье я объясню, почему, как мне кажется, дублирование не является причиной всех бед, и почему совершенно нормально бывает иногда повторяться.
Какую задачу должен был решать принцип DRY?
В 1999 году Энди Хант и Дейв Томас выпустили книгу «Программист-прагматик» (The Pragmatic Programmer). С тех пор она стала невероятно популярной, а множество уроков из этой книги стало повсеместной практикой для большого количества разработчиков ПО.
Одна из практик называлась Don’t Repeat Yourself (DRY). Вот определение DRY из «Программиста-прагматика»:
Каждый элемент информации должен иметь единственное недвусмысленное авторитарное представление в системе.
Мне это кажется абсолютно логичным. Иными словами, это значит, что ваша бизнес-логика не должна повторяться в двух местах.
Краткий пример DRY в действии
Допустим, вы работаете над системой электронных продаж, в которой заказ действителен в течение семи дней. Логика того, истёк ли срок заказа, должна определяться только один раз. Например, в классе Order:
class Order
def initialize(order_id:, order_timestamp:, items:)
@order_id = order_id
@order_timestamp = order_timestamp
@items = items
end
def expired?
Time.zone.now > order_timestamp + 7.days
end
endКак видите, логика истечения срока заказа находится внутри класса Order. Если это единственное место, где размещается такая логика, код в этом репозитории можно считать отвечающим DRY.
Однако если где-то в другом месте есть второй фрагмент кода, использующий ту же логику, то код больше не будет соответствовать DRY, так как бизнес-правило дублируется в двух местах.
Это неоптимально, потому что если бизнес-правило необходимо будет изменить так, чтобы срок заказов истекал через пять дней, то придётся менять его в нескольких местах. В такой ситуации будет невероятно просто забыть одно из таких мест, из-за чего в разных частях приложения применится разная логика.
Более того, бизнес-логика должна принадлежать сущностям предметной области, а не просто плавать по разным частям кодовой базы. Логично, что если вы хотите понять логику истечения срока заказов, то перейдёте в класс
Order.Второй фрагмент кода нужно отрефакторить, чтобы он использовал метод
expired? из Order для соответствия принципу DRY.Пока всё в порядке. Я описал то, для чего должен предназначаться DRY, но в последнее время его применяют гораздо более произвольно, и это наносит ущерб коду.
Проблема со слишком ревностным применением DRY
Теперь мы знаем, для чего нужен принцип DRY и даже как правильно его применять.
Однако, как говорилось ранее, DRY используется так не всегда. Я видел множество комментариев к пул-реквестам, в которых ошибочно утверждалось, что код должен быть изменён для соответствия DRY, когда на самом деле дублирование использовалось с хорошей целью.
Достаточно часто DRY применяют к любому дублированию, а не конкретно к дублированию бизнес-правил и знаний. Ситуация часто ухудшается и инструментами автоматизированного анализа кода, которые выделяют любое дублирование как плохой код.
Пример, в котором DRY будет неприменим
Мне кажется, лучшим способом объяснения разницы между хорошим и плохим подходом является пример, поэтому покажу два класса с небольшим дублированием. Код взят из реального проекта, однако немного упрощён.
class OrderPlacedEventPublisher
def publish(order)
publisher = EventPublisher.for_event('orer_placed')
.with_data(order_id: order.id, items: order.items)
add_location_context(location_uuid: order.location_uuid, publisher: publisher)
publisher.publish
end
def add_location_context(location_uuid:, publisher:)
return if location_uuid.blank?
publisher.with_context(
name: 'location_context',
data: {
location_uuid: location_uuid
},
version: '1-0-0'
)
end
end
class ItemViewedEventPublisher
def publish(item)
publisher = EventPublisher.for_event('item_viewed')
.with_data(item_id: item.id)
add_location_context(location_uuid: item.location_uuid, publisher: publisher)
publisher.publish
end
def add_location_context(location_uuid:, publisher:)
return if location_uuid.blank?
publisher.with_context(
name: 'location_context',
data: {
location_uuid: location_uuid
},
version: '1-0-0'
)
end
endВ репозитории это два отдельных файла, я поместил их в один блок кода для простоты. Оба класса публикуют событие в Kafka в совершенно разных частях приложения и в разных ситуациях.
Каждое событие имеет основной раздел (например,
order_placed ) и опционально список контекстов для предоставления вспомогательной информации, часто применимой к нескольким событиям (например, к местоположению пользователя, которое не сохраняется в товарах и заказах в проекте, но здесь это к делу не относится).Я внёс изменение, добавляющее контекст в обоих событиях, после чего инструмент анализа кода выделил это дублирование как проблему; так же поступил мой коллега в пул-реквесте.
Пожалуйста, повторяйтесь
В этом случае я не согласен ни с инструментом анализа кода, ни с проверяющим. Хотя здесь есть дублирование, нет дублирования ни бизнес-правил, ни знаний.
К сожалению, я не могу найти конкретный доклад, но впервые я услышал фразу «Пожалуйста, повторяйтесь» на конференции O’Reilly в Берлине, и она сильно запала мне в душу.
В этом примере, разумеется, можно убрать дублирование, например, перенеся метод
add_location_context во вспомогательный модуль и включив этот модуль в каждый из классов. На Ruby это можно было бы сделать так:module LocationContextHelper
def add_location_context(location_uuid:, publisher:)
return if location_uuid.blank?
publisher.with_context(
name: 'location_context',
data: {
location_uuid: location_uuid
},
version: '1-0-0'
)
end
end
class OrderPlacedEventPublisher
include LocationContextHelper
... остальная часть класса ...
end
class ItemViewedEventPublisher
include LocationContextHelper
... остальная часть класса ...
endВы будете правы, сказав, что дублирование устранено, но ценой добавления общей зависимости для обоих классов. В полном проекте есть множество контекстов в разных частях кодовой базы, поэтому для соответствия DRY в добавлении контекстов к событиям нам, вероятно, понадобится пять вспомогательных классов (или один большой вспомогательный класс, что ещё хуже!), которые нужно будет включить примерно в пятнадцать классов публикаторов событий.
Цена устранения такого дублирования, на мой взгляд, слишком высока. В методе
add_location_context нет бизнес-логики, это простой код, который всего лишь требуется в совершенно несвязанных друг с другом частях кодовой базы.Устранив дублирование, мы без необходимости связали вместе большое количество классов. Если в контекст понадобится добавить дополнительные данные, то, разумеется, это быстрее сделать, если есть вспомогательный класс, но нет гарантии, что нужные данные находятся в текущих передаваемых параметрах; а в таком случае вам всё равно придётся изменять все места, в которых используется вспомогательный класс.
Подведём итог
Надеюсь, эта статья показалась вам полезной, и возможно, когда в следующий раз вы решите удалить какое-то дублирование, вы задумаетесь, имеет ли смысл это удаление.
Стоит заметить, что мы обсуждали DRY только в контексте одного репозитория. Если вы, например, работаете с архитектурой микросервисов, то более вероятно, а иногда и разумно, иметь дублирование части бизнес-логики в нескольких сервисах. Но это уже тема для отдельной статьи.