# Calculation in Concern module using Rails

I wrote the following Concern module. Is it good practice to perform all the calculations in the included scope? How can it be improved?

module Pagination
module Helper
extend ActiveSupport::Concern

included do
class << self
def paged(params, custom_per_page = nil, custom_page = nil)
per_page = params[:per_page] =
custom_per_page || normalize_per_page(params[:per_page].to_i)
page = normalize_page(self.all.size,
custom_page || params[:page].to_i, per_page)

self.paginate(page: page, per_page: per_page)
end

def default_per_page
end

protected

def normalize_per_page per_page
if per_page < 1 || per_page > default_per_page
return default_per_page
end
per_page
end

def normalize_page(size, page, per_page)
if ((page - 1) * per_page) >= size
page = (size / per_page.to_f).ceil
end
page > 0 ? page : 1
end
end
end
end
end

ActiveRecord::Base.send(:include, Pagination::Helper)


Since you're mixing methods into ActiveRecord::Base, you might as well extend it proper:

class ActiveRecord::Base
def self.paged(...)
...
end
end


No concerns necessary; you're just extending a class. Put it into a file in config/initializers and it'll load when the app spins up.

About what you're mixing in:

• default_per_page doesn't make sense as a mixin. It's not the responsibility of every ActiveRecord-derived class to have a shortcut for fetching a global config value. The methods you add to a class via mixins should make as much sense, as if you'd added them directly to just that class.
I'd place shortcuts like that in e.g. a separate helper module, and call it there.

• Use Rails.configuration instead of <YourAppName>.config if you're aiming for reusable code.

• Consider using hashes for method parameters. It makes calling your methods simpler and more descriptive.

• Be consistent with your parentheses. You declaration of normalize_per_page omits the parentheses, but your declaration of normalize_page includes them. Personally, I always use parens in method declarations.

• Do not access params in a model. Keep that in the controller, and only the controller; it's its job. This also means keeping the normalize_page method (which I'd call constrain_page_number, because it doesn't normalize a page, but constrains a number) in the controller. You can simply add it to ApplicationController to have available:

# in app/application_controller.rb

protected

def constrain_page_number(number, record_count, per_page = nil)
per_page ||= PagniationHelpers.default_per_page # our helper module
[1, number, (record_count / per_page).floor + 1].sort[1]
end


However, in my view, your controller should send a redirect if the page number in out of bounds - don't just rewrite it behind the scenes. So you might want something like this instead:

def valid_page_number?(number, record_count, per_page = nil)
per_page ||= PagniationHelpers.default_per_page # our helper module
max = (record_count / per_page).floor + 1
(1..max).cover?(number)
end

• Now, something that'd might make more sense to include in every model would be a max_page method. It'd allow you to more easily check the page number in the controller.
Of course, such a method would (unlike the above) have to rely on just the default scope when counting the number of records. Still, it might be useful.

In all I end with something like this (plus the controller method above):

module PagniationHelpers
module_function

def default_per_page
Rails.config.custom.default_per_page_pagination
end
end

class ActiveRecord::Base
class << self
def paged(page, per_page = nil)
per_page ||= PagniationHelpers.default_per_page
self.paginate(page: page, per_page: per_page)
end
end
end


Buuuut of course, it looks like you're using the will_paginate gem. In that case, forget the code above. Instead, do this in an initializer file somewhere:

WillPaginate.per_page = 10 # or whatever default you want


and in ApplicationController do:

def max_page_number(record_count, per_page = nil)
per_page ||= WillPaginate.per_page
(record_count / per_page).floor + 1
end

def valid_page_number?(number, record_count, per_page = nil)
max = max_page_number(record_count, per_page)
(1..max).cover?(number)
end

def constrain_page_number(number, record_count, per_page = nil)
max = max_page_number(record_count, per_page)
[1, number, max].sort[1]
end


And in your controller actions you can do something like

def index
page = params[:page].try(:to_i) || 1
unless valid_page_number?(page, Post.count)
redirect_to posts_path(page: constrain_page_number(page, Post.count))
end
@posts = Post.paginate(:page => page)
#...
end