Hi,
In a view, I have this code :
<% MeasureUnitDecorator.all.each do |mu| %>
<p>
<b>Dimensions in <%= mu.title %> :</b> <%= painting.convert(:width, mu) %> x <%= painting.convert(:height, mu) %>
<% if painting.depth %>
x <%= painting.convert(:depth, mu) %>
<% end %>
</p>
<% end %>
<% MeasureUnitDecorator.all.each do |mu| %>
<p>
<b>Dimensions with frame in <%= mu.title %> :</b> <%= painting.convert(:width_with_frame, mu) %> x <%= painting.convert(:height_with_frame, mu) %>
<% if painting.depth %>
x <%= painting.convert(:depth_with_frame, mu) %>
<% end %>
</p>
<% end %>
The result is this this :
<p><b>Dimensions in cm :</b> 10 x 14 x 16</p>
<p><b>Dimensions with frame in cm :</b> 12 x 16 x 18</p>
It work but I think there is too many logic in a view and I have a lot of duplications.
I tried to refactor this code with a decorator like this (in this view : ) :
<% MeasureUnitDecorator.all.each do |mu| %>
<%= painting.dimensions_in(mu) %>
<% end %>
And in a decorator :
#....
def dimensions_in(mu)
properties = [:width, :height, :depth]
dimensions = formated_dimensions(properties)
h.content_tag(:p, "<b>Dimensions in #{mu.title} :</b> #{dimensions}".html_safe)
end
def dimensions_with_frame_in(mu)
properties = [:width_with_frame, :height_with_frame, :depth_with_frame]
dimensions = formated_dimensions(properties)
h.content_tag(:p, "<b>Dimensions with frame in #{mu.title} :</b> #{dimensions}".html_safe)
end
private
def formated_dimensions(properties)
properties.map { |p| convert(p, mu) }.compact.join(" x ")
end
def convert(property, unit)
source_unit = model.measure_unit.title
target_unit = unit.title.to_sym
depth.send(source_unit).convert(target_unit).to_f.round(2)
end
#....
It’s cleaner but I think the decorator have too many responsabilities so I did this (in a presenter file) :
class DimensionPresenter
attr_accessor :painting, :measure_unit, :source_unit, :target_unit
def initialize(painting, measure_unit)
@painting = painting
@measure_unit = measure_unit
end
def format(properties)
properties.map { |p| convert(p, mu) }.compact.join(" x ")
end
def source_unit
@source_unit ||= painting.measure_unit.title
end
def target_unit
@target_unit ||= measure_unit.unit.title.to_sym
end
def convert(property, unit)
depth.send(source_unit).convert(target_unit).to_f.round(2)
end
end
And I use it in my decorator like this :
def dimensions_in(mu)
properties = [:width, :height, :depth]
formated_dimensions = DimensionPresenter.new(painting, mu).format(properties)
h.content_tag(:p, "<b>Dimensions in #{mu.title} :</b> #{dimensions}".html_safe)
end
def dimensions_with_frame_in(mu)
properties = [:width_with_frame, :height_with_frame, :depth_with_frame]
formated_dimensions = DimensionPresenter.new(painting, mu).format(properties)
h.content_tag(:p, "<b>Dimensions with frame in #{mu.title} :</b> #{dimensions}".html_safe)
end
It’s ok but I don’t like to pass a model as argument in a decorator function. A decorator function should use only the decorated model.
I can also use a presenter like this :
<% MeasureUnitDecorator.all.each do |mu| %>
<%= DimensionPresenter.new(painting, mu).format %>
<% end %>
<% MeasureUnitDecorator.all.each do |mu| %>
<%= DimensionPresenter.new(painting, mu).format_with_frame %>
<% end %>
But I don’t like to initialize an object in a view. I can also create an instance variable in the controller but I don’t want to pass many instance variable. It’s not in the Sandy Metz best practices (http://robots.thoughtbot.com/post/50655960596/sandi-metz-rules-for-developers).
I also think to an helper by I think an help method but be generic and should can be used by every models. This is not the case for this because the is only a painting with this attributes.
So… I don’t know what to do. It’s a very little thing but I don’t find the solution.
What do you think? Do you have a better solution?
Thanks!
