Why I think an "open" decorator is mostly a bad idea

README.md

Why I think an "open" decorator is mostly a bad idea

First thing first: when I speak about open decorators I mostly refer to the delegate_all (anti)pattern introduced in many Rails applications by the widely used draper gem. With "open" adjective I refer to the fact that the decorator responds to all the messages as the underlying decorated object. This is like a transparent ill variation of the proxy pattern: if you need proxy pattern¹ then you want to re-bound the interface, lowering required knowledge as much as you can, lowering responsibilities and lowering the maintenance of boundaries between objects and application layers as much as you are able to.

Draper is a (sort of) presenter

While the family of decoration patterns² is large and with subtle differences between one member and another, I consider Draper's decorators as decorators for the view layer, thus we could speak more of a presenter³. The readme of the gem itself says

you decorate the article before handing it off to the view

so we're aligned.

On the other hand if we'd need a generic object decorator without any additional whistle we could, and should I'd say, use ruby's Forwardable that's at our fingertips.

Assumed we've a presenter, we also should assume that a presenter should respond only to a set of messages useful for the view layer. Why an object responsible of rendering-related behaviours should ever respond to model-related messages? Let's go with an example

class Model < ApplicationRecord
  has_one :relation
  has_one :author
  has_many :relations
  
  after_commit :run_after_commit
  
  def recalculate
    update_columns foo: CalculatorService.call(self)
  end
end

def ModelDecorator < Draper::Decorator
  delegate_all

  def title
    object.title.upcase
  end
end

class ModelsController < ApplicationController
  def show
    render :show, locals: { model: ModelDecorator.decorate(Model.find(params[:id])) }
  end
end

# app/views/models/show.html.slim
article
	h1= model.title
	span.byline= [model.author.first_name, model.author.last_name].join(' ')
	p= model.relation.content

Notable pains:

1. the view knows about entity relationship with other models and the front-end dev hijacked this knowledge creating a hard couple with something far away from its responsibilities
2. any changes in any place will probably involve an update in three different files
3. view is able to access any attribute of the model
4. view is even able to call mode.recalculate at its will
5. you don't know what the view needs without reading through all the partials
6. you don't know what the controller is offering to the view without reading all the model and its relationships (good luck)

Generally speaking you've completely lost control of the information flow within your application and you're not just hard coupling two objects, but three/four entire layers each others.

Talk only with neighbours

Given we're using a presenter, our view should have only one neighbour: the presenter. The only possible communication must be between neighbour objects.

def ModelDecorator < Draper::Decorator
  delegate :content, :author_fullname

  def title
    object.title.upcase
  end
end

class Author < ApplicationRecord
  def fullname
    "#{first_name} #{last_name}".freeze
  end
end

class Model < ApplicationRecord
  has_one :relation
  has_one :author
  has_many :relations
  
  after_commit :run_after_commit
  
  delegate :content, to: relation
  delegate :fullname, to: :author, prefix: true
  
  def recalculate
    update_columns foo: CalculatorService.call(self)
  end
end

# app/views/models/show.html.slim
article
  h1= model.title
  span.byline= model.author_fullname
  p= model.content

1. the view doesn't know about entity relationships: it's a show in a CRUD and it knows about its entity
2. you can refactor any place in isolation without breaking interfaces. Relation could change what content is and the information will be propagated throughout the objects messaging chain. You could present it differently in the decorator/presenter simply by removing the delegation and adding a method instead
3. the view cannot see over nor through its own friend the presenter. If you need something then you'll have to expose it and consume it specifically
4. the presenter doesn't have behaviours, doesn't leak business logics and cannot access the DB
5. the presenter is now the single point you have to read in order to know what view and model needs to communicate each other
6. problem solved in point number 5 :)

Isn't delegate a transparent proxy itself?

No: delegate explicitly proxy one message. Transparent proxying means to proxy any message w/o even controlling the signature.

Footnotes

1. https://en.wikipedia.org/wiki/Proxy_pattern ↩

2. https://en.wikipedia.org/wiki/Decorator_pattern ↩

3. https://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93presenter ↩