2016-05-27 1 views
0

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

current_user.follows.each do |follow| 
    follow.followable.open_notifications.each do |notification| 
     if !notification.reads.include?(current_user) 
      @open_notifications += notification 
     end 
    end 
end 

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

OpenNotification Контроллер:

OpenNotification.fetch_unread(current_user) 

OpenNotification Модель:

def self.fetch_unread(user) 
    @open_notifications = [] 
    user.follows.each do |follow| 
     follow.followable.open_notifications.each do |notification| 
      if !notification.reads.include?(user) 
       @open_notifications += notification 
      end 
     end 
    end 
    @open_notifications 
end 

EDIT:

Классы участвуют:

  • Пользователь
  • Follow - кто следует (пользователь), что (followable)
  • Followable (полиморфный - может быть пользователь, или событие)
  • OpenNotification - хранит информацию об изменении followable объекта
  • Read - кто читает, какие уведомления (user_id и open_notification_id)

Пользователь:

has_many :follows, class_name: 'Follow', 
       source: :user 

has_many :follows_as_fallowable, 
       class_name: 'Follow', 
       as: :followable 

has_many :followers, through: :follows_as_fallowable, 
        source: :user 

Событие:

has_many :follows, as: :followable 
has_many :followers, through: :follows, 
         source: :user 

has_many :open_notifications, as: :followable 

OpenNotification:

belongs_to :followable, polymorphic: true 
has_many :reads 

Читает:

belongs_to :user 
belongs_to :open_notification 

Мой вопрос в том, является ли хорошей практикой относиться к нескольким классам класса, ответственного за один конкретный ресурс?
Если это не очень хорошая практика, как код должен быть реорганизован?

+0

Меня не интересует использование разных классов (это похоже на мои, как будто есть только два класса: «Пользователь» и какой-то «последователь»). Но ваши вложенные петли 'each' приведут к плохой производительности, потому что она запускает множество запросов к базе данных (проблема N + 1). Я бы сосредоточил свое внимание на этом. Без знания схемы базы данных и ассоциаций моделей трудно дать рекомендации. Можете ли вы указать, как связаны классы? – spickermann

+0

Отношения довольно сложны, я добавил информацию (см. Править) и краткое объяснение. – Mathew

ответ

0

Ссылка на несколько моделей из одной модели совершенно прекрасна до тех пор, пока вы можете с полным основанием сказать, что ваша модель уведомления должна знать модель пользователя (в основном, большинство взаимодействий с уведомлениями касается и пользователей). В том маленьком, что вы показываете здесь, я бы сказал, что вам нужно будет рассмотреть этот вопрос и оценить его. Если вы считаете, что это может быть немного растянуто, я бы рекомендовал внедрить эту логику в отдельный класс, чтобы ваш код был более удобным и предсказуемым, если бы кто-то сотрудничал с вами в этом (думаю, объект службы).Кроме того, если вы добавляете уведомления в @open_notifications, вам следует использовать <<, так как каждый раз он не будет восстанавливать объект @open_notifications, тогда как += будет повторять его снова и снова, оставляя больше работы для сборщика мусора. На больших операциях вы увидите здесь значительное увеличение скорости.

 Смежные вопросы

  • Нет связанных вопросов^_^