2
\$\begingroup\$

I use ruby 1.8.7. I have method wich select records and fill hash. In my opinion the code is not dry. Could somebody help me to refactor it and make it shorter and more elegant?

def fill_attributes()
  if (!$array[:status_id])
            $array[:status_id] = @customer.status_id
             @status_name=Status.find($array[:status_id]).name rescue nil
            $array[:status]=@status_name if @status_name                           
      end

     if (!$array[:color_id])
            $array[:color_id] = @customer.color_id
            @color_name=Color.find($array[:color_id]).name rescue nil
            $array[:color]=@color_name if @color_name
      end
     .....

  end  

My version is:

  map={'model1' => Model1, 'model2' => Model2,'model3' =>Model3}

  map.keys.each do |model|   
       key="#{model}_id".to_sym
       value="#{model}.#{model}_id" 
     if (!$array[key])  
          $array[key] =  #instance_variable_set("@#{value}",eval(value, binding) ) 
          name=map[model].find($array[key]).name rescue nil
          $array[key]=name if name                  
      end
   end

But there's a problem, to use instance_variable_set I have to create a class? What is the best way to refactor it?

\$\endgroup\$

migrated from stackoverflow.com Mar 19 '12 at 12:20

This question came from our site for professional and enthusiast programmers.

1
\$\begingroup\$

I think you can optimize it like :

def fill_attributes()
  if (!$array[:status_id])
    $array[:status_id] = @customer.status_id
    $array[:status]= get_attribute_name('status',$array[:status_id] )
  end

  if (!$array[:color_id])
    $array[:color_id] = @customer.color_id
    $array[:color]= get_attribute_name('color',$array[:color_id] )
  end

end 

Private

def get_attribute_name(relator, relator_id)
  relator = relator.classify.constantize.find(relator_id)
  !relator.nil? ? relator.name : nil
end

Hope this will help you .

\$\endgroup\$
  • \$\begingroup\$ Hi, but it does not reduced the code :) almost the same \$\endgroup\$ – alexeyb Mar 19 '12 at 10:13
  • 3
    \$\begingroup\$ Refactoring isn't always about making your code shorter. Your aim is always to make the code easier to understand, or to introduce more flexibility so that you can add a new feature. A refactor often has the effect on increasing your code length (especially if you extract lots of methods). This is not a bad thing if your code becomes easier to read. \$\endgroup\$ – sgmorrison Mar 19 '12 at 21:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy