Подсчета абонентов с соответствующими почтовый индекс, интересы и секс


Я пытаюсь цикл через несколько из этих блоков. Они в основном сузить число людей, которые выполняют кучу атрибутов.

Прошу прощения, если это выглядит действительно грязно, но моя база данных действительно негативно сказываются обработку этого, и я знаю, что есть лучший путь. Я просто потерял на стратегию прямо сейчас.

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 << cs.id
      end
    end
  end

  # any matching interests?..
  # interest
  @topics = self.distributions.map(&:me_topic).compact
  unless  @topics.nil? || @topics.empty?
    @matched_topics = MeTopic.all.map(&:name) & @topics
    @matched_topics.each do |mt|
      MeTopic.find(:all, :conditions => ["name = ?", mt]).each do |mt2|
        mt2.users.each do |u|
          array_of_users << u.card_signup.id if u.card_signup
        end
      end
    end
  end

  # any matching sexes?..
  # sex
  @sexes = self.distributions.map(&:sex).compact
  unless @sexes.nil? || @sexes.empty?
    @matched_sexes = CardSignup.all.map(&:sex) & @sexes
    @matched_sexes.each do |ms|
      CardSignup.find(:all, :conditions => ["sex = ?", ms]).each do |cs|
        array_of_users << cs.id
      end
    end
  end

  total_number = array_of_users.compact.uniq

  return total_number
end


385
3
задан 1 апреля 2011 в 07:04 Источник Поделиться
Комментарии
3 ответа

Первое, что я заметил, это то, что ты называешь карте.{}.компактный , когда вы, вероятно, могли быть призвание выберите{}, то есть, вы хотите выбрать самостоятельно.распределение. Так что бы заменить 6 способ звонков с 3.

Если вы используете Select, он всегда возвращает массив, и никогда не пропустит, поэтому вместо того, чтобы , если Х. Х. nil или пустой, вы могли бы использовать , если х. Есть?, что бы заменить 6 способ звонков с тремя.

Если вы не используете @matched_zips вне этого контекста, нет смысла присваивать переменной здесь. Вместо назначения, вы могли бы просто использовать (CardSignup.все.карта(&:zip_code) & @zip_codes).каждый сделает, что бы избавить тебя три переменных назначений.

6
ответ дан 1 апреля 2011 в 07:04 Источник Поделиться

Другое дело, что может быть исправлено-это обратный вызов. Поэтому вместо того, чтобы

def count_of_distribution 
# ... your code ...

total_number = array_of_users.compact.uniq

return total_number
end

Вы могли бы снять total_number переменной и вместо того, чтобы писать:

def count_of_distribution 
# ... your code ...

array_of_users.compact.uniq
end

4
ответ дан 11 апреля 2011 в 06:04 Источник Поделиться

В дополнение к тому, чего уже упомянул, я заметил, что вы сидите много переменных экземпляра. Очевидно, я не знаю, как и где вы собираетесь использовать их, но мой первый инстинкт-это признак плохого дизайна.

Вообщем метод, который называется в первую очередь для его возвращаемое значение, следует избегать побочных эффектов (например, установка переменных экземпляра), когда это возможно.

Еще один момент: можно использовать набор вместо массива. Таким образом, вы не должны называть уник на нем.

3
ответ дан 24 апреля 2011 в 01:04 Источник Поделиться