Archive for the ‘bad smell’ Category

Ruby idioms : Avoid check class membership

Avoid use class, is_a? and kind_of? to check class membership. Replace these statement with a message to the object that respond to the message. The code below is an anti-pattern.

if mortgage.class==FixRateMortgage
   mortgage.change_rate_not_allowed
else
   mortgage.rate=rate
end

The code below is more idiomatic in Ruby.

class Mortgage
  attr_accessor :rate
end 

class FixedRateMortgage < Mortgage
  def initialize()
    @rate=5
  end
  def rate=(rate)
    change_rate_not_allowed
  end

  def change_rate_not_allowed
     puts "The rate in a fixed rate mortage can not be change"
  end
end

Sending the method rate= to either kind of classes will result in the appropriate behaviour.

Commons bugs in ruby: blocks and local variables

Originally blocks did not have truly local variables. The block
parameters were really local variables in the enclosing method.

x=0
1.upto 100 do |z|
  x = x+z
end       
x  #=> 5050 

Commons bugs in ruby: Changing collection while iterating over it

You should never, never, never iterate over a collection which the iteration loop somehow modifies. Elements of the collection will be moved during the iteration. Elements might be missed or handled twice.

 b=(1..10).collect # => [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
 b.each do |elem|
   b.delete elem
 end #=> [2, 4, 6, 8, 10]

Two snippets on how to refactor a ruby application using block helpers.

Poor coding approach :
1, Insert HTML code as strings

<%= link_to "<strong>#{product.name}</strong><span>
            #{pluralize(product.topic_count(company), 'topic')}</span>",
    href, :class => "product_label" %>

2. Use condinional sentences to display content :

 <% if @user.rol == :admin %>
   Section only for admins
 <%end >

Solutions,

1. Replace the old lin_to with a block helper (via educate, liberate):

<% link_to browse_url(product), :class => "product_label" do %>  <%= product.name %>

(<%= pluralize(product.topic_count(company), 'topic') %>)

<% end %>

2. Use a code block helper to insolate the rol administation logic :

 <% @user.admin? do %>
   section only for admins
  <%end%>

Bad Smells sign in rails code.

  1. Calling find on a model direct
    1
    2
    3
    4
    %  Sales.find_all_by_region(params[:region]) 
                                    .each do |t| %>
    
      # some inteligent code
    
    <% end %>
  2. alling find on an association
  3. Conditionally inserting content if/else clauses, case statements, etc
    1
    <% if current_user.admin? %>Admins see this<% end %>
  4. Doing a complex inline map, sort, select, etc on a collection
  5. 1
    
    2
    <% applications.sort_by{|app| [app.priority, app.creator.name]}
                                  .each do |app|
    <% end %>
  6. assigning temporary variables
    1
    
    2
    
    3
    
    4
    
    5
    
    
    <% indicator_id = "loading_indicator_#{item.id}" %>
    
        <%= link_to_remote("Show Status",:url => item_path(item),
    
            :loading => "$('#{indicator_id}').show()",
    
            :complete => "$('#{indicator_id}').hide()",) %>
    
    <%= image_tag('progress.gif', :id => indicator_id)%>
    
    

When V is for Vexing: Patterns to DRY Up Your Views

Follow

Get every new post delivered to your Inbox.