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          Reader::Application.config.custom.default_per_page_pagination        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  endendActiveRecord::Base.send(:include, Pagination::Helper)
Calculation in Concern module using Rails
ruby;ruby on rails
Since you're mixing methods into ActiveRecord::Base, you might as well extend it proper:class ActiveRecord::Base  def self.paged(...)    ...  endendNo 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.rbprotecteddef 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]endHowever, 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)endNow, 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  endendclass 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  endendBuuuut 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 wantand in ApplicationController do:def max_page_number(record_count, per_page = nil)  per_page ||= WillPaginate.per_page  (record_count / per_page).floor + 1enddef valid_page_number?(number, record_count, per_page = nil)  max = max_page_number(record_count, per_page)  (1..max).cover?(number)enddef constrain_page_number(number, record_count, per_page = nil)  max = max_page_number(record_count, per_page)  [1, number, max].sort[1]endAnd in your controller actions you can do something likedef 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