3
\$\begingroup\$

I use Ruby 1.8.7. I have a method which selects records according to the item that I pass to the method.

In my opinion, the code is not DRY. Could somebody offer suggestions on refactoring it to make it shorter?

def get_record (id,item)
  case item
   when "category"
      @temp = Category.find(id)
   when "status"
      @temp = Status.find(id)
   when "industry"
     @temp = Industry.find(id)
   end
 return @temp.name if @temp
end
Improve this question
\$\endgroup\$
4
  • \$\begingroup\$ I'd say the code is a sample of bad programming, but I fail to see the relation with DRY principles. \$\endgroup\$
    – Candide
    Mar 13, 2012 at 13:58
  • \$\begingroup\$ What the method should return when @temp is nil? \$\endgroup\$
    – megas
    Mar 13, 2012 at 14:01
  • \$\begingroup\$ before I pass Item i m checking it for nil? and empty string. \$\endgroup\$
    – alexeyb
    Mar 13, 2012 at 14:17
  • \$\begingroup\$ I aagree with you Ingenu , but code leading to bugs and anty dry principles :) \$\endgroup\$
    – alexeyb
    Mar 13, 2012 at 14:25

1 Answer 1

Reset to default
6
\$\begingroup\$

You want to use classify and constantize:

require 'set'
AUTHORIZED = Set[Product, Status, Industry]
def get_record(id, item)
  model = item.classify.constantize
  raise "Go away, hacker" unless AUTHORIZED.include? model
  model.find(id).name
end

Be careful about security, as without the unless this will access any class in your system. This is just an example.

Improve this answer
\$\endgroup\$
2
  • \$\begingroup\$ Marc-André Lafortune, incase this => CLASSES.fetch(item.to_sym).find(id) will be nil ? maybe is better to use CLASSES.fetch(item.to_sym).find(id).name rescue "" ? \$\endgroup\$
    – alexeyb
    Mar 13, 2012 at 14:32
  • \$\begingroup\$ find(id) will raise an exception if the record can not be found. Use find_by_id if you want nil in those cases. \$\endgroup\$
    – Marc-André Lafortune
    Mar 14, 2012 at 14:23