views:

45

answers:

3

So, I have this block of code, and yes it looks ugly. Is there a better a data structure I can use to compare row.add,row.modify,row.delete and row.query with "Y" and call the get_role function? Note this block of code runs in a loop, hence the 'row'.

    if row.add == "y"
      role_ids << get_role("c")  
    end
    if row.modify == "y"
      role_ids << get_role("u")
    end
    if row.delete == "y"
      role_ids << get_role("d")
    end
    if row.query == "y"
      role_ids << get_role("r")
    end

Thanks !

+2  A: 
role_ids << case
  when row.add    == "y" then get_role("c")
  when row.modify == "y" then get_role("u")
  when row.delete == "y" then get_role("d")
  when row.query  == "y" then get_role("r")
end
Simone Carletti
A: 

UPDATE !

changed to

operations = [row.add,row.modify,row.delete,row.query]
crud=["c","u","d","r"]
operations.each_with_index{|operation,i| if operation == "y" then role_ids << get_role(crud[i]) end}
Shreyas Satish
that will always trigger `get_role("y")`. right track, but not quite.
Jeriko
Thanks for the heads-up Jeriko. I think now the changed version will work.
Shreyas Satish
This solution is really unreadable. It also introduces additional variables with the cost of more objects to be created.
Simone Carletti
Did it really deserve a -1 !?
Shreyas Satish
+2  A: 
operations = { :add => "c", :modify => "u", :delete => "d", :query => "r" }
operations.each do |key,value|
  role_ids << get_role(value) if row.send(key) == "y"
end
Jeriko
When you call `row.send(:method_name)`, it's the same as calling `row.method_name`. It's necessary because your operations / method names are stored in an array (because they need corresponding values). For example, `object.delete` and `object.send(:delete)` are identical. Does that help explain?
Jeriko
http://ruby-doc.org/core/classes/Object.html#M000332
Jeriko
Yes I figured , thats why I deleted the comment. Thanks !
Shreyas Satish
This technique is called *Dynamic Dispatch*.
Lars Haugseth
Just a small remark: prior to 1.9, Ruby's Hash wasn't ordered, so there's no guarantee that `each` here would traverse the hash in the same order it was defined. If the order is important, one should use array of arrays (pairs) instead of a hash, with no change to the `each` call needed.
Mladen Jablanović
Good point @Mladen, thanks.
Jeriko