Method to count unread messages

I currently use this method to count unread messages for a user (self here):

def unread_messages_count
  number = 0
  conversations = Conversation.where {|conversation| conversation.user1 == self || conversation.user2 == self }
  conversations.each do |conversation|
    number += conversation.messages.select{|message| !message.read && (message.user != self) }.count
  end
  number
end

However, this seems highly inefficient to me since I'm iterating on every conversation found to look for unread messages.

Any input on how to make this method smarter will be much appreciated !

About your code:

-
As a general rule, the imperative pattern acc = 0; xs.each { |x| acc += f(x) }; total = acc should be replaced by the functional expression total = xs.reduce(0) { |acc, x| acc + f(x) } or total = xs.map { |x| f(x) }.reduce(0, :+). Given that Rails provides an Enumerable#sum, it can be simplified even more: total = xs.map { |x| f(x) }.sum.

-
That said, this is not the way you do things in Rails, you should use associations and scopes. The code will be much faster (all the processing is performed by the database engine) and more reusable.

I'd write:

class User 
  has_many :conversations, ->(user) { Conversation.with_user(user) }
  has_many :messages, through: :conversations

  def unread_messages_count
    messages.not_sent_by(self).unread.count
  end
end

class Conversation
  scope :with_user, ->(user) { where("? IN (user1_id, user2_id)", user.id) }
end

class Message
  scope :unread, -> { where(read: false) }
  scope :not_sent_by, ->(user) { where.not(user: user) }
end

Note that we could write the where-condition of with_user directly in the has_many block, but this way we respect encapsulation: the User model knows nothing about the attributes of the Conversation table.

StackExchange Code Review Q#133754, answer score: 10