patternrubyrailsMinor
Counting subscribers with matching ZIP code, interests, and sex
Viewed 0 times
countingwithandsexzipinterestscodesubscribersmatching
Problem
I am trying to cycle through a few of these blocks. They basically narrow down a number of people that fulfill a bunch of attributes.
I apologize if this seems really messy, but my database is really taking a toll processing this, and I know there's a better way. I'm just lost on strategy right now.
I apologize if this seems really messy, but my database is really taking a toll processing this, and I know there's a better way. I'm just lost on strategy right now.
def count_of_distribution
#beginning with an array..
array_of_users = []
# any matching zip codes? ..
# zip_codes
@zip_codes = self.distributions.map(&:zip_code).compact
unless @zip_codes.nil? || @zip_codes.empty?
@matched_zips = CardSignup.all.map(&:zip_code) & @zip_codes
@matched_zips.each do |mz|
CardSignup.find(:all, :conditions => ["zip_code = ?", mz]).each do |cs|
array_of_users ["name = ?", mt]).each do |mt2|
mt2.users.each do |u|
array_of_users ["sex = ?", ms]).each do |cs|
array_of_users << cs.id
end
end
end
total_number = array_of_users.compact.uniq
return total_number
endSolution
The first thing I notice is that you're calling
If you use select, it always returns an array and never a nil, so instead of
If you aren't using
map.{}.compact when you probably could be calling select{}, that is, you want to select from self.distribution. So that would replace 6 method calls with 3.If you use select, it always returns an array and never a nil, so instead of
unless X.nil or X.empty, you could use if X.any?, which would replace 6 method calls with three.If you aren't using
@matched_zips outside of this context, it makes no sense to assign a variable here. Instead of assigning, you could just use (CardSignup.all.map(&:zip_code) & @zip_codes).each do, which would spare you three variable assignments.Context
StackExchange Code Review Q#1611, answer score: 6
Revisions (0)
No revisions yet.