Rails - Jul 2013 - Refactor a view by a decorator, a presenter, an helper or something else?

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!

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web visit
https://groups.google.com/d/msgid/rubyonrails-talk/CABfX5Pa%2Bzz0gmNCfYOofyqqie_H9r8772pmjqacn6bQ7BN-9fA%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.