← Back to Upcase

Code review custom model


(Alvin) #1

Folowing some advices in Intermediate Ruby on Rails trail, i did it some changes to a personal dashboard controller putting the logic in a custom model.

But I think the model doesn’t smell well.

I have the same dashboard view for an user and admin.
The main difference is the admin can see all the data in the view and normal user just see his data.

##Controller

class DashboardController < ApplicationController
  
  layout "dashboard/dashboard"
  
  def show
    @dashboard = Dashboard.new(current_user)
  end
end

##Model

class Dashboard
  attr_reader :user

  def initialize(user)
    @user = user
  end
  
  def total_patient
    if user.has_role? :admin
      Patient.count
    else
      user.total_patient
    end
  end
  
  def total_consultations
    if user.has_role? :admin
      Consultation.count
    else
      user.total_consultations
    end
  end
  
  def consultations
    if user.has_role? :admin
      Consultation.all
    else
      Consultation.last_consultations(user)
    end 
  end
end

I think I am repeating the same if user.has_role? :admin in all methods and maybe I can apply some pattern for this behavior.


(Andy Waite) #2

From the code shared above, it looks you could have two kinds of dashboards, e.g. AdminDashboard and NormalUserDashboard, both of which have the same interface.

In the controller you could use a factory to return the appropriate kind of dashboard, e.g.

@dashboard = DashboardFactory.dashboard_for(current_user)

(Alvin) #3

Thanks. It makes sence have a DashboardFactory that return the appropiate dashboard object.