Decorator pattern for an application

Question

I've created a decorator in my Rails application. However, the initializer has 3 params. Is this generally acceptable in decorator or should I refactor this or try a different pattern?

class FileBinderDecorator
 attr_reader :binder, :credential, :page
 def initialize(binder, credential, page = 1)
 @binder = binder
 @credential = credential
 @page = page
 end
 def file_items
 file_items = credential.file_item_list(excluded_ids: binder.file_items.pluck(:id))
 file_items.page(page).per(7)
 end
 def method_missing(method_name, *args, &block)
 @binder.send(method_name, *args, &block)
 end
 def missing_responds_to?(method_name, include_private = false)
 @binder.respond_to?(method_name, include_private) || super
 end
end

manage.html.slim

.modal.binder-modal.fade data-selected-ids=[] [id="edit-files-modal"]
 .modal-dialog
 .modal-content
 .modal-header
 button.close aria-label="Close" data-dismiss="modal" type="button"
 span aria-hidden="true" ×
 h4.modal-title Edit Files
 .modal-body
 .container-fluid
 .row-fluid
 .credentials
 = select_tag 'credentials', options_from_collection_for_select(credentials_for_current_user_and_account(current_user, @account), :first, :last)
 .selectors
 .file-box-select-all
 = link_to 'Select All', '#'
 .file-box-deselect-all
 = link_to 'Deselect All', '#'
 .row-fluid
 .list-group[class="binder-#{decorator.id}"]
 = render partial: 'file_binders/filebox', locals: { file_items: decorator.file_items, binder: decorator.binder, credential: decorator.credential }
 .row-fluid
 .text-center
 .pagerx[class="pager-#{decorator.id}"]
 = render partial: 'file_binders/pager', locals: { file_items: decorator.file_items, binder: decorator.binder, credential: decorator.credential }
 .modal-footer
 button.add-selected.btn.btn-primary class="add-selected-#{decorator.id}" data-dismiss='modal' data-count=0 data-file-items=[]
 | Remove Selected
 button.btn.btn-cancel aria-label="Close" id="modalCloseButton#{decorator.id}" data-dismiss="modal" type="button" aria-hidden="true" Cancel

Answer 1

The whole idea behind decorators, proxies and drapers is that they encase a single object. Yours seems to be serving two masters and has far too much knowledge of the different objects in your system.

It hard to get a picture of your models from the question - but it seems like you are missing a key relationship since you are passing binder and credential around together.

Also instead of creating your own proxying methods you can simply extend the Delegator class but first you have to figure what you are actually proxying and stick to that object.

• Antarr Byrd: There is no direct relationship between binder and credential. I binders can have many files. And these files can belong to any credential that belong the same account as the binder.

Answer 2

To expound on @papirtiger's answer re using the Delegator class, consider this meta programming...

class FileBinderDecorator
 def method_missing(method_name, *args, &block)
 # ... 
 def missing_responds_to?(method_name, include_private = false)
 # ...
end

Can be reduced to this...

class FileBinderDecorator < SimpleDelegator
 def initialize(binder)
 super(binder)
 end
end

Then you don't need to pass the credential to the initialize method. But you can should you need to.

def file_items_for(credential)
 credential.file_item_list(excluded_ids: file_items.pluck(:id)).page(page).per(7)
end

Your view will be simplified too. Instead of calling decorator.bind you can treat the decorator as the binder object.

render partial: '...', locals: {
 file_items: decorator.file_items, binder: decorator ... }

I would take this a step further and pass in the view context.

 def initialize(binder, context)
 @context = context
 super(binder)
 end

And now you can initialize this from the view:

- binder = FileBinderDecorator.new(@binder, self)

Ideally you will want a nice helper for this. Now you can helpers from inside the decorators using the context.

= binder.render_file_box_partial
def render_file_box_partial
 @context.render partial: 'file_binders/filebox', ...
end

You might want to wrap context in a helper for brevity. The convention is h, for helper.

 def h
 @context
 end
 def some_tag
 h.content_tag ...
 end

As you can see, a lot can be done to spruce this up.