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