Skip to content

Conversation

@cyprus11
Copy link

@cyprus11 cyprus11 commented Mar 7, 2025

No description provided.

@@ -0,0 +1,9 @@
class BooksController < ApplicationController
def index
books = Book.all.order(:created_at).limit(::Settings.app.items_per_page)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно опустить all

Book.order(:created_at).limit(::Settings.app.items_per_page)

def index
books = Book.all.order(:created_at).limit(::Settings.app.items_per_page)
page = params[:page].to_i
books = books.offset(page * ::Settings.app.items_per_page) if page > 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Еще немного смущает BooksController#index и вот этот участок кода:

books = Book.all.order(:created_at).limit(::Settings.app.items_per_page)
page = params[:page].to_i
books = books.offset(page * ::Settings.app.items_per_page) if page > 1

Может, конечно, у меня уже деформация ), но просится упаковать это в метод вида

pager(Book, params[:page].to_i)

Но заранее согласен, что тут три строки и выделение такого объема в метод или сервис-объект избыточно, нам просто придется нырять в новый файл, чтобы узнать что там происходит. Возможно дело в слишком длинном и повторяющемся ::Settings.app.items_per_page и достаточно его просто упаковать в отдельный private-метод

def per_page
  ::Settings.app.items_per_page
end

и оставшийся код станет более компактным

books = Book.order(:created_at).limit(per_page)
page = params[:page].to_i
books = books.offset(page * per_page) if page > 1

Плюс хочется все-таки определение books в одну строку уложить... например мы можем под page выделить метод

def page
  [params[:page].to_i, 0].max
end

тогда этот участок можно переписать так

books = Book.order(:created_at)
          .limit(per_page)
          .offset(page * per_page)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants