Simple: Need help refactoring this awkward looking method I need help refactoring this method. It is way too long.

  def report_total(feed_event, advisor)
    count = 0
    advisor.activity_feed_events.each do |lead|
      if lead == SignupFeedEvent
        count += 1
      else
        if lead.is_a?(feed_event) 
          if lead.event_date &gt; (Time.now - 7.days)
            count += 1
          end
        end
      end
    end
    count
  end
You could use scopes to do this at the database level, which would (potentially) be much faster than returning then iterating over all ActivityFeedEvents.

# ActivityFeedEvent
scope :by_type, -&gt;(type){ where(type: type.to_s) }
scope :recent, -&gt;{ where("event_date &gt; ?", Time.now - 7.days) }

# wherever report_total is
def report_total(feed_event_type, advisor)
  advisor.activity_feed_events.by_type(SignupFeedEvent).count +
    advisor.activity_feed_events.recent.by_type(feed_event_type).count
end


Note that you need a lambda for the recent scope despite it not taking args, because otherwise the Time.now will only be evaluated once (when the server starts).

Or, perhaps even more neatly, you could do it the other way round:

# ActivityFeedEvent
scope :for, -&gt;(advisor){ where(advisor_id: advisor.id) }
scope :recent, -&gt;{ where("event_date &gt; ?", Time.now - 7.days) }

# wherever report_total is
def report_total(feed_event_type, advisor)
  SignupFeedEvent.for(advisor).count + feed_event_type.for(advisor).count
end


Depending on your models you may need a more descriptive name than for for this one, belonging_to or for_advisor are candidates.

Also note that some people prefer to write scopes as class methods instead of using lambdas

# ActivityFeedEvent
def self.for(advisor)
  where(advisor_id: advisor.id)
end

def self.recent
  where("event_date &gt; ?", Time.now - 7.days)
end