Select records and fill hash

Question

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?

Answer 1

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 .

• alexeyb: Hi, but it does not reduced the code :) almost the same
• sgmorrison: 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.