# Nicer way to work params [closed]

if params[:package]
city_ids = []

city = params[:package].delete 'city'
city_ids << city  if city.to_i > 0

cities = params[:package].delete 'cities'
city_ids += cities  if cities.class == Array && cities.length > 0

params[:package][:cities] = city_ids.map{ |c| City.get(c) }.compact
end


This code looks pretty clean to me. Here are a few thoughts:

I'm not sure what your ORM is (or even that City is database backed), but most have a way to fetch more then one record at a time. Making a single call will save time and Database resources.

Another thought is that you are inducing a lot of local vars: city_ids, city, cities. What about trying to build up the array of ids in params[:package][:cities]? e.g.

#if cities is not an array, it is bad date get rid of it.
params[:package].delete 'cities' unless params[:package][:cities].kind_of? Array
params[:package][:cities] ||= []
params[:package][:cities] << (params[:package].delete 'city') if params[:package][:cities].to_i > 0
params[:package][:cities] = params[:package][:cities].map{ |c| City.get(c) }.compact


The code above doesn't seem very DRY because it has all this params[:package] stuff. If you do things like this a lot (your question makes it sound like you do) factor the operation out into a separate method:

def extract_nested_reference(hash,values) # change name as appopriate
if hash
hash.delete values unless hash[values].kind_of? Array
hash[values] ||=[]
hash[values] << hash[values.singularize] if hash[values.singularize].to_i > 0

hash[values] = hash[values].map do |x|
Object.const_get(values.singularize.constantize).get(c)
end.compact
end
end


This method could then be called with extract_nested_reference(params[:package],'cities'). Refactoring like this will also encourage you to be consistent in how you build you params hash, and allow you to reuse code.

Finally, your question asked about better ways of dealing with hashes. I don't think there is. You will find code very similar to this living in most popular gems. AS you say, it is kind of clunky, and therefore and excellent candidate for a code redactor.

• You are right about ORM. I'm usind marvellous DataMapper and it invokes just one sql statement instead of city_ids.count with this code: City.all(:id => city_ids.compact) – ujifgc Apr 21 '11 at 19:40