HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyrailsMinor

Decorator pattern for an application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
decoratorforpatternapplication

Problem

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 }

Solution

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.

Context

StackExchange Code Review Q#94564, answer score: 2

Revisions (0)

No revisions yet.