Improving JavaScript code in a view file

Question

I am using Ruby on Rails 3.0.7 and I would like to know how I can refactor, DRY (Don't Repeat Yourself) and improve the following code in my view file:

<% articles.each do |article| %>

  <div>
    <%= link_to 'add', '#', :id => "create_#{article.id}" %>
  </div>

  <script type="text/javascript">
    # Code - Block 1
    $jQ('#create_<%= article.id %>').live('click', function(event) {
      $jQ.ajax({
        type:    "POST",
        url:     "<%= article_url(@article) %>/create",
        data:    "article_id=<%= @article.id %>",
        error:   function(jqXHR, textStatus, errorThrown) {
          alert(textStatus + ' - ' + errorThrown + '\n\n' + jqXHR.responseText);
        },
        success: function(data, textStatus, jqXHR) {
          $jQ('#create_<%= article.id %>').replaceWith('<%= escape_javascript("added") + (link_to 'remove', '#', :id => "destroy_#{article.id}") %>')
        }
      });
    });

    # Code - Block 2
    $jQ('#destroy_<%= .id %>').live('click', function(event) {
      $jQ.ajax({
        type:    "POST",
        url:     "<%= article_url(@article) %>/destroy",
        data:    "article_id=<%= @article.id %>,
        error:   function(jqXHR, textStatus, errorThrown) {
          alert(textStatus + ' - ' + errorThrown + '\n\n' + jqXHR.responseText);
        },
        success: function(data, textStatus, jqXHR) {
          $jQ('#destroy_<%= article.id %>').replaceWith('<%= escape_javascript("removed") + (link_to 'add', '#', :id => "create_#{article.id}") %>')
        }
      });
    });
  </script>

<% end %>

Note:

The each statement!
The "two code blocks" above are almost equal.

vol7ron · Accepted Answer · 2011-07-07 11:52:53Z

1

Put the main body in a string
Have an array of arrays for two iterations (one with ['create','added','remove',destroy'] and the other with ['destroy','removed','add','create'])
Loop twice (0..1), interpolating your array values for those slots where the text is found
- First Pass: array[0][0], array[0][0], array[0][0], array[0][1], array[0][2], array[0][3]
- Second Pass: array[1][0], array[1][0], array[1][0], array[1][1], array[1][2], array[1][3]

Note: my Ruby is rusty, so here's the pseudo code.
Example (look for ary[i][n]):

ary = [['create','added','remove',destroy'],
       ['destroy','removed','add','create']]

for i in (0..1)
   $jQ('#**ary[i][0]_<%= article.id %>').live('click', function(event) {
      $jQ.ajax({
         type:    "POST",
         url:     "<%= article_url(@article) %>/ary[i][0]",
         data:    "article_id=<%= @article.id %>",
         error:   function(jqXHR, textStatus, errorThrown) {
                     alert(textStatus + ' - ' + errorThrown + '\n\n' + jqXHR.responseText);
                  },
         success: function(data, textStatus, jqXHR) {
                     $jQ('#ary[i][0]_<%= article.id %>')
                        .replaceWith('<%= escape_javascript("ary[i][1]") + (link_to 'ary[i][2]', '#', :id => "ary[i][3]_#{article.id}") %>')
                  }
      });
   });
end

edited Jul 7, 2011 at 11:52

answered Jul 6, 2011 at 22:28

vol7ron

42.3k22 gold badges126 silver badges178 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Backo Over a year ago

I didn't understand your approach at all (maybe because I am a newbie). What do you mean with 'array[0][0], array[0][0], array[0][0], ...' and 'array[1][0], array[1][0], array[1][0], ...'? How should I "loop" for array[loopCountVar]?

vol7ron Over a year ago

I meant, you'll be calling the first array element three times in your string. I don't remember any Ruby, so I wasn't going to supply the example code, but I found my statement confusing, so I put some pseudocode.

vol7ron Over a year ago

you would put the above in something like for i in (0..1)...end. I've edited the code above to demonstrate. Note: the loop and array would be Ruby-code - I'll let you figure out how the interpolation goes.

spd · Accepted Answer · 2011-07-06 23:58:32Z

0

What you can do is nest another array of methods inside of your article loop, and use it to tweak the inner code block for each method accordingly.

Here's an example: (untested but should be close)

<% articles.each do |article| %>

  <div>
    <%= link_to 'add', '#', :id => "create_#{article.id}" %>
  </div>

  <script type="text/javascript">
    <% [:create,:destroy].each do |method| %>
          $jQ('#<%="#{method}_#{article.id}" %>').live('click', function(event) {
            $jQ.ajax({.   
              type:    "POST",
              url:     "<%= "#{article_url(@article)}/#{method}" %>",
              data:    "article_id=<%= @article.id %>",
              error:   function(jqXHR, textStatus, errorThrown) {
                alert(textStatus + ' - ' + errorThrown + '\n\n' + jqXHR.responseText);
              },
              success: function(data, textStatus, jqXHR) {
                $jQ('#<%="#{method}_#{article.id}" %>').replaceWith('<%= escape_javascript({'create'=>'added','destroy'=>'removed'}[method]) + (link_to( {'create'=>'add','destroy'=>'remove'}[method], '#', :id => "#{method}_#{article.id}")) %>')
              }
            });
          });

    <% end %>
  </script>
<% end %>

Your other options are to turn this into a helper method that takes an article and a method and generates the correct output, or you can put the necessary data in the html tags and have javascript convert it.

edited Jul 6, 2011 at 23:58

answered Jul 6, 2011 at 23:48

spd

1,7761 gold badge11 silver badges7 bronze badges

2 Comments

Marten Veldthuis Over a year ago

I think I would loop wtih [[:create, 'added'], [:destroy, 'removed']].each do |method, message|, just to keep the definitions all in one place.

vol7ron Over a year ago

The above is what I tried to say. The thing to take away, is yes, you were duplicating your code, but you might come to find that duplication was necessary down the line as the two events could turn out to be very different w/ callbacks and other stuff - keep that in mind. While it's nice to reduce lines, it's not always beneficial. Looping will also require more processing (time).

Collectives™ on Stack Overflow

Improving JavaScript code in a view file

2 Answers 2

3 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related