55d6457963d5be7ad341343105ddc8a3

Here is a function I have built which places a bunch of radio buttons within a row of a table. Basically, you pass in a name for them and a caption along with a list of possible values. This helper is used to build a list of 20 questions in a table.

I want to call it like this:

<%= place_radio_button_row(:name => '1', :caption => '1. How much do you love rails?', :values => ['Somewhat', 'Moderately', 'A lot', 'Not at all']) %>

and then I want it to generate HTML that looks like this onto the page

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 # Sample Output:
 #
 #  <tr>
 #     <td>1. How much do you love rails?</td>
 #     <td></td>
 #     <td><input type="radio" name="1" id="1" value="Somewhat" /></td>
 #     <td><input type="radio" name="1" id="1" value="Moderately" /></td>
 #     <td><input type="radio" name="1" id="1" value="A lot" /></td>
 #     <td><input type="radio" name="1" id="1" value="Not at all" /></td>
 #  </tr>
 #
 # op_hash = {:name => '1', :caption => '1. How much do you love rails?',
 #            :values => ['Somewhat', 'Moderately', 'A lot', 'Not at all'] }

 def place_radio_button_row(op_hash = {})
   concat( tag(:tr,nil,true) + "\n", binding)
   concat( "\t" + content_tag(:td, op_hash[:caption]), binding)
   op_hash[:values].each do |value|
     concat( "\n\t" , binding)
	 concat( content_tag(:td,radio_button_with_owner_tag(op_hash[:name], value)), binding)
   end
   concat("\n</tr>\n", binding)
end

Refactorings

No refactoring yet !

E635ccff7389d9070f5e7e9fe8b36beb

rpheath

November 18, 2007, November 18, 2007 04:59, permalink

No rating. Login to rate!

I don't believe you need to use #concat here, since you're not passing in a block. Also, I personally wouldn't worry about how the markup is output (ie, considering the "\n\t" insertions all over the place). If you worried about that in all of your helpers that created html, your code would become cluttered and hard to read, when it's really not that important. Of course, that's just my opinion.

Here's how I would do this:

1
2
3
4
5
6
7
def place_radio_button_row(options={})
  returning html = "<tr>" do
    html << "<td>#{options[:caption]}</td><td></td>"
    options[:values].each { |v| html << "<td>#{radio_button_with_owner_tag(options[:name], v)}</td>"
    html << "</tr>"
  end
end
4d1c9dad17af98e55cb65b4efce27c42

Ben Burkert

November 18, 2007, November 18, 2007 23:06, permalink

No rating. Login to rate!

I like to use nested blocks to handle the closing of tags automatically.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
def tag(name, attributes = {}, &block)
  attributes = attributes.to_a.collect { |a| "#{a.first}=\"#{a.last}\""}.join(' ')
  inner = block ? "\n\t#{block.call}\n" : ""
  "<#{name}>#{inner}</#{name}>"
end

def place_radio_button_row(options={})
  tag(:tr) do
    html = tag(:td) { options[:caption] }
    html << tag(:td)
    html << options[:values].each do |value|
      tag(:td) do
        radio_button_with_owner_tag(options[:name], value)
      end
    end
  end
end

Your refactoring





Format Copy from initial code

or Cancel