0
\$\begingroup\$

The idea is to return a number of messages received today, this week, this month.

  def index
    @mailboxes = current_user.mailboxes
    @today, @month, @week, @all_time = 0, 0, 0, 0

    @mailboxes.each do |mailbox|
      @today += mailbox.messages.today.length
      @month += mailbox.messages.week.length
      @week += mailbox.messages.month.length
      @all_time += mailbox.messages.length
    end
  end

week, month scopes are similar to the following one

  scope :today, -> { where(created_at: ((Time.zone.now - 24.hours)..Time.zone.now)) }
\$\endgroup\$
1
\$\begingroup\$

Some suggestions ...

Use a counter cache for messages on Mailbox -- the count of all can be read from the mailbox instance.

Use #size instead of #length. This will use a counter cache for messages on Mailbox if one is present, or will use a SQL count. #length will get all of the messages back, then check the size of the array.

I would be concerned about having multiple mailboxes. If your user model has_many :messages, through: :mailbox you can run these queries more efficiently:

messages = current_user.messages

@today    = messages.today.size
@week     = messages.week.size
@month    = messages.month.size
@all_time = messages.size

With a counter cache, you're only running three queries to get the counts, and thy should be efficient enough.

Place a single index on the columns (user_id, id) on the mailboxes tables, and an index on (mailbox_id, created_at) on the messages table.

Edit: I just noticed the week and month variable assignments were round the wrong way. Also, if you wanted to be slightly more efficient you could define a scope for week_except_today and month_except_week, etc, and then:

@today    = messages.today.size
@week     = @today + messages.week_except_today.size
@month    = @week  + messages.month_except_week.size
@all_time = messages.size
\$\endgroup\$
1
\$\begingroup\$

As David points out you are going to run four queries. The size method will actually run more efficient sql, and not load the objects into memory which is also more efficient if you don't need the actual objects.

Also scope :today, -> { where(created_at: ((Time.zone.now - 24.hours)..Time.zone.now)) } does not print out todays' messages but rather those from the last 24 hours. You could use the beginning_of_day method to get the start of today.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.