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.
- Calling find on a model direct
1 2 3 4 % Sales.find_all_by_region(params[:region]) .each do |t| %> # some inteligent code <% end %>- alling find on an association
- Conditionally inserting content if/else clauses, case statements, etc
1 <% if current_user.admin? %>Admins see this<% end %>- Doing a complex inline map, sort, select, etc on a collection
1 2 <% applications.sort_by{|app| [app.priority, app.creator.name]} .each do |app| <% end %>- 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)%>