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.

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.