I have index action in my Controller:
def index
authenticate_admin!
@users = User.paginate(per_page: 25, page: params[:page])
if params[:list_type].to_i == 2 # => Pending mentors
@users = @users.where(documents: { is_verified: 0 }, user_type: 0, profile_status: 1)
elsif params[:list_type].to_i == 1 # => approved listing
@users = @users.where(documents: { is_verified: 1 }, user_type: 0) if params[:user_type].to_i.zero? # => Mentor
@users = @users.where(user_type: 1) if params[:user_type].to_i == 1 # => mentee
end
@users = @users.where('full_name LIKE ?', "%#{params[:search]}%") if params[:search].present?
@users = if params[:sort].to_i == 1
@users.order(full_name: :asc)
else
@users.order(id: :desc)
end
@users = @users.profession.where(id: params[:profession_ids]) if params[:profession_ids].present?
@users = @users.call('age >= ?', params[:from_date]) if params[:from_date].present?
@users = @users.call('age <= ?', params[:to_date]) if params[:to_date].present?
@users = @users.where(gender: params[:gender]) if params[:gender].present?
@users = @users.includes(:document)
end
but when I run Rubocop
to check if my code has any offense
.
then it returns me two errors.
app/controllers/users_controller.rb:5:3: C: Metrics/CyclomaticComplexity: Cyclomatic complexity for index is too high. [11/6]
def index ...
^^^^^^^^^
app/controllers/users_controller.rb:5:3: C: Metrics/PerceivedComplexity: Perceived complexity for index is too high. [13/7]
def index ...
^^^^^^^^^
So, I divide my index action to multiple actions like:
def index
authenticate_admin!
@users = User.paginate(per_page: 25, page: params[:page])
if params[:list_type].to_i == 2 # => Pending mentors
@users = @users.where(documents: { is_verified: 0 }, user_type: 0, profile_status: 1)
elsif params[:list_type].to_i == 1 # => approved listing
@users = @users.where(documents: { is_verified: 1 }, user_type: 0) if params[:user_type].to_i.zero? # => Mentor
@users = @users.where(user_type: 1) if params[:user_type].to_i == 1 # => mentee
end
search
end
def search
@users = @users.where('full_name LIKE ?', "%#{params[:search]}%") if params[:search].present?
@users = if params[:sort].to_i == 1
@users.order(full_name: :asc)
else
@users.order(id: :desc)
end
apply_filter
end
def apply_filter
@users = @users.profession.where(id: params[:profession_ids]) if params[:profession_ids].present?
@users = @users.call('age >= ?', params[:from_date]) if params[:from_date].present?
@users = @users.call('age <= ?', params[:to_date]) if params[:to_date].present?
@users = @users.where(gender: params[:gender]) if params[:gender].present?
@users = @users.includes(:document)
end
Is this the right way to implement code like this or is there anything else i can do to refine my coding structure?
Is this the right way to implement code like this
As long as you find the code reasonable and clear. If the index
method is the only method in the controller, then it won't be a problem to navigate through the logic of the users search.
is there anything else i can do to refine my coding structure
One of the options would be extracting the these two methods (or even more logic) into a separate class/file. For example, something like:
controller
def index
authenticate_admin!
@users = User.paginate(per_page: 25, page: params[:page])
if params[:list_type].to_i == 2 # => Pending mentors
@users = @users.where(documents: { is_verified: 0 }, user_type: 0, profile_status: 1)
elsif params[:list_type].to_i == 1 # => approved listing
@users = @users.where(documents: { is_verified: 1 }, user_type: 0) if params[:user_type].to_i.zero? # => Mentor
@users = @users.where(user_type: 1) if params[:user_type].to_i == 1 # => mentee
end
@users = Users::Search.new(@users).call
end
service
class Users::Search
attr_reader :params, :users
def initialize(users, params)
@users = users
@params = params
end
def search
users = @users.where('full_name LIKE ?', "%#{params[:search]}%") if params[:search].present?
users = if params[:sort].to_i == 1
users.order(full_name: :asc)
else
users.order(id: :desc)
end
apply_filter(users)
end
def apply_filter(users)
users = users.profession.where(id: params[:profession_ids]) if params[:profession_ids].present?
users = users.call('age >= ?', params[:from_date]) if params[:from_date].present?
users = users.call('age <= ?', params[:to_date]) if params[:to_date].present?
users = users.where(gender: params[:gender]) if params[:gender].present?
users.includes(:document)
end
end
And one of the advantages of the extracting is that such class is easier to test, than a controller method, which requires a request and parsing a response for every test case.