patternrubyrailsMinor
Decorator pattern for an application
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?
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 }
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
endmanage.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
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.
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.