TIL not obsessively trying to be DRY
POSTED ON:
Quite often, DRY is applied to any duplication, rather than specifically to the duplication of business rules and knowledge. This is often made worse by automated code analysis tools, which will often highlight any duplication as a code smell.
The example used to explain why obsessively trying to DRY is a bad idea.
These are two separate files:
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
Each event has a main section (e.g. order_placed ) and optionally, a list of contexts to provide supplementary information that often applies to multiple events (e.g. the location of the user — which isn’t stored on the items and orders in the project, but that’s not relevant here).
You’d be right in saying the duplication has been removed but at the cost of introducing a shared dependency to both classes.
By removing the duplication, we’ve unnecessarily coupled a large number of classes together. If you needed to add additional data to the context, sure it would be quicker if there was the helper, but there’s no guarantee the data needed is in the parameters currently passed, in which case you have to update all places that the helper is used anyway.
Via the post https://betterprogramming.pub/a-case-against-relying-solely-on-dry-16dd712e422b
A similar post is from Dan Abramov and "Goodbye Clean code": https://overreacted.io/goodbye-clean-code/
Related TILs
Tagged: cleancode