Drying up Code

How can I refactor the following code? I know that the code from the Ruby for Rails book was meant to illustrate some concepts. I am just curious to know how I can utilize the dynamic capabilities of Ruby. TIA.

def link_to_composer(composer)
  link_to(composer.whole_name,
  :controller => "composer",
  :action => "show",
  :id => composer.id)
end

def link_to_edition(edition)
  link_to edition.description,
  :controller => "edition",
  :action => "show",
  :id => edition.id
end

def link_to_edition_title(edition)
  link_to edition.nice_title,
  :controller => "edition",
  :action => "show",
  :id => edition.id
end

def link_to_work(work)
link_to(work.nice_title,
:controller => "work",
:action => "show",
:id => work.id)
end

def link_to_instrument(instrument)
  link_to instrument.name,
  :controller => "instrument",
  :action => "show",
  :id => instrument.id
end

It’s pretty nice already.

If the classes are named accordingly (i.e. composer is a Composer
object, edition is an Edition object) then you can do this:

link_to_item(item, field)
  link_to(item.send(field),
    :controller => item.class.name.underscore,
    :action => 'show',
    :id => item.id)
end

Then, the link_to_composer method would look like this:

def link_to_composer(composer)
  link_to_item(composer, :whole_name)
end

Hope that helps!

-David Felstead

I don’t think spreading that knowledge (the field name to show for particular model) is a good idea. It almost eliminates the advantage of using custom link helpers and error prone.

Ahh… sorry. Missed the second part.
Yeah, that kind of generalization is a good thing. Although, I would made it more flexible:

def link_to_item(item, title_attribute, options = {})

link_to(item.send(title_attribute), {
:controller => item.class.name.underscore,
:action => ‘show’,
:id => item.id
}.merge(options))
end

This will allowading extra params to url_for and overriding default ones. You could even add html_options as fourth param or use technique as in link_to_remote (where url params go to options[:url], html params - options[:html] etc)

hi david

that was nice

regards
gaurav

Personally I'd have the link_to_item method and the link_to_composer,
link_to_edition methods, just to enforce DRY within the helpers
themselves.

It's all down to semantics and personal preference, IMHO - whatever
floats your boat.

Bala Paranj wrote:

How can I refactor the following code? I know that the code from the Ruby for Rails book was meant to illustrate some concepts. I am just curious to know how I can utilize the dynamic capabilities of Ruby. TIA.

def link_to_composer(composer)
  link_to(composer.whole_name,
  :controller => "composer",
  :action => "show",
  :id => composer.id)
end

def link_to_edition(edition)
  link_to edition.description,
  :controller => "edition",
  :action => "show",
  :id => edition.id
end

def link_to_edition_title(edition)
  link_to edition.nice_title,
  :controller => "edition",
  :action => "show",
  :id => edition.id
end

def link_to_work(work)
link_to(work.nice_title,
:controller => "work",
:action => "show",
:id => work.id)
end

def link_to_instrument(instrument)
  link_to instrument.name,
  :controller => "instrument",
  :action => "show",
  :id => instrument.id
end

First, make a method in each of those models called 'title_for_links',
which returns the appropriate bit of text. That'll standardise all of
those different method calls.

Then you could have something like:

def link_to_object(object)
  link_to(object.title_for_links, :controller =>
object.class.name.downcase, action => :show, :id => object.id)
end

assuming that the appropriate controller is always a lowercase version
of the object's class name.

Chris

Bala Paranj wrote:

How can I refactor the following code?

Could "named routes" be an easier way to do it? Also if you stick with your current method what about using "define_method" to do some meta-programming. So you can turn your code into:

link_to_helper :composer
link_to_helper :edition
.....

then just define "link_to_helper" which uses "define_method" to generate the methods for you.

Eric

Hi --

Hi --

If the classes are named accordingly (i.e. composer is a Composer
object, edition is an Edition object) then you can do this:

link_to_item(item, field)
  link_to(item.send(field),
    :controller => item.class.name.underscore,
    :action => 'show',
    :id => item.id)
end

Then, the link_to_composer method would look like this:

def link_to_composer(composer)
  link_to_item(composer, :whole_name)
end

I don't think spreading that knowledge (the field name to show for
particular model) is a good idea. It almost eliminates the advantage of
using custom link helpers and error prone.

It's not really spreading the knowledge; the starting point is
presumably:

   link_to @composer.whole_name ...

   link_to @edition.nice_title

and so on. I think it's reasonable to abstract whatever is the same
between these different cases, and parameterize whatever isn't.

David

It depends on how nuts you want to go. This code should create a
bunch of link_to methods that take a link_text and object argument
(different from yours which only appears to take an object argument).
I've defined a stub link_to method and some added a few lines of code
to see if this really works. Let me know if this helps answer the
question.

BTW: Another way to solve the problem is create the method using
method_missing, although you have to be careful because Rails relies
on this to some extent for its magic.

class Foo
   %w(composer edition edition_title work instrument).each do |thingie|
     src = <<-END_SRC
     def link_to_#{thingie}(link_text, #{thingie})
       self.link_to(link_text,
       :controller => "#{thingie}",
       :action => "show",
       :id => #{thingie}[:id])
     end
     END_SRC
     class_eval src, __FILE__, __LINE__
   end

   def link_to(text, options = {})
     puts "link text is [#{text}] and link variables are:"
     options.each_pair{|option, value| puts "#{option}: #{value}"}
   end
end

foo = Foo.new
foo.link_to_composer('composer test', {:id => 1})
foo.link_to_edition('edition test', {:id => 2})

Hi --

How can I refactor the following code? I know that the code from
the Ruby for Rails book was meant to illustrate some concepts. I am
just curious to know how I can utilize the dynamic capabilities of
Ruby. TIA.

def link_to_composer(composer)
  link_to(composer.whole_name,
  :controller => "composer",
  :action => "show",
  :id => composer.id)
end

def link_to_edition(edition)
  link_to edition.description,
  :controller => "edition",
  :action => "show",
  :id => edition.id
end

def link_to_edition_title(edition)
  link_to edition.nice_title,
  :controller => "edition",
  :action => "show",
  :id => edition.id
end

def link_to_work(work)
link_to(work.nice_title,
:controller => "work",
:action => "show",
:id => work.id)
end

def link_to_instrument(instrument)
  link_to instrument.name,
  :controller => "instrument",
  :action => "show",
  :id => instrument.id
end

It depends on how nuts you want to go. This code should create a
bunch of link_to methods that take a link_text and object argument
(different from yours which only appears to take an object argument).
I've defined a stub link_to method and some added a few lines of code
to see if this really works. Let me know if this helps answer the
question.

BTW: Another way to solve the problem is create the method using
method_missing, although you have to be careful because Rails relies
on this to some extent for its magic.

class Foo
  %w(composer edition edition_title work instrument).each do |thingie|
    src = <<-END_SRC
    def link_to_#{thingie}(link_text, #{thingie})

You don't have to use thingie as the local variable in the method
(although I did so in the written-out versions, to match the
ActiveRecord class names); you can use any variable name, and then
reuse it in the :id line below.

      self.link_to(link_text,
      :controller => "#{thingie}",
      :action => "show",
      :id => #{thingie}[:id])
    end
    END_SRC
    class_eval src, __FILE__, __LINE__
  end

Another possibility, slightly more concise:

    %w(composer edition edition_title work instrument).each do |thingie|
      define_method("link_to_#{thingie}") do |link_text,obj|
        link_to(link_text,
          :controller => thingie,
          :action => "show",
          :id => obj[:id])
      end
    end

David