Rails: вспомогательный рефакторинг - PullRequest
1 голос
/ 12 августа 2011

У меня есть помощник, который мне кажется смешным, но я не смог придумать, как его улучшить. Вот этот помощник:

# Shows Admin Menu Button
def admin_toggle_button
  if user_signed_in? && ( current_user.has_role?(:admin) || ( @collection && can?(:curate,@collection) ) )
    if session[:admin_menu] == :on
      link_to( 'Admin Tools', edit_shared_path(:admin_menu => :off), :remote=>true, :class => 'selected', :id => 'admin_toggle_button', :title => 'Hide Admin Menu' )
    else
      link_to( 'Admin Tools', edit_shared_path(:admin_menu => :on), :remote=>true, :id => 'admin_toggle_button', :title => 'Show Admin Menu' )
    end
  end
end

В строке меню моего приложения я вызываю admin_toggle_button, и этот помощник определяет, должна ли присутствовать эта кнопка и каково ее состояние.

Для присутствия кнопки меню администратора должен быть зарегистрированный пользователь, и этот пользователь должен быть администратором ИЛИ пользователю необходимо просматривать коллекцию, которую ему разрешено курировать (редактировать).

Мой вопрос: являются ли такие вспомогательные методы нормальными - то есть вы находите, что время от времени вам нужны такие сложные методы - или я что-то упускаю? Можете ли вы предложить способ улучшить этот метод?

Ответы [ 2 ]

3 голосов
/ 12 августа 2011

Вы можете создать метод для первого условия, чтобы вы могли использовать его в другом месте.

def can_view_admin_stuff?
  user_signed_in? && ( current_user.has_role?(:admin) || ( @collection && can?(:curate,@collection) ) )
end

def admin_toggle_button
  return '' unless can_view_admin_stuff?

  if session[:admin_menu] == :on
    link_to( 'Admin Tools', edit_shared_path(:admin_menu => :off), :remote=>true, :class => 'selected', :id => 'admin_toggle_button', :title => 'Hide Admin Menu' )
  else
    link_to( 'Admin Tools', edit_shared_path(:admin_menu => :on), :remote=>true, :id => 'admin_toggle_button', :title => 'Show Admin Menu' )
  end
end

Но да, это нормально, я думаю, найти методы, особенно если вы используете условие толькоссылка на сайт.Если вы используете его для других частей вашего кода, было бы неплохо иметь помощника для него.

0 голосов
/ 12 августа 2011

Этот метод не очень сложен.Самая сложная часть - проверка доступа, и даже это не так уж плохо.Одна из причин, по которой помощники должны скрывать подобные вещи, чтобы вы не делали ничего плохого.

Повторение между двумя вариантами link_to может потребовать некоторого внимания.Вы можете немного перестроить его, но:

def admin_toggle_button
  return '' if !user_signed_in? || !( current_user.has_role?(:admin) || ( @collection && can?(:curate,@collection) ) )
  opts = {
    :id     => 'admin_toggle_button',
    :remote => true,
    :title  => 'Show Admin Menu'
  }
  admin_menu = :on
  if session[:admin_menu] == :on
    opts[:title] = 'Hide Admin Menu'
    opts[:class] = 'selected'
    admin_menu   = :off
  end
  link_to('Admin Tools', edit_shared_path(:admin_menu => admin_menu), opts)
end

Этот подход подчеркивает различия между двумя возможными link_to вызовами.Если session[:admin_menu] != :on встречается чаще, возможно, вы захотите изменить логику, чтобы запустить opts и admin_menu с настройками «Скрыть меню администратора», а затем настроить их на случай != :on при необходимости.

def admin_toggle_button
  return '' if !user_signed_in? || !( current_user.has_role?(:admin) || ( @collection && can?(:curate,@collection) ) )
  opts = {
    :class  => 'selected',
    :id     => 'admin_toggle_button',
    :remote => true,
    :title  => 'Hide Admin Menu'
  }
  admin_menu = :off
  if session[:admin_menu] != :on
    opts[:title] = 'Show Admin Menu'
    opts.delete(:class)
    admin_menu   = :on
  end
  link_to('Admin Tools', edit_shared_path(:admin_menu => admin_menu), opts)
end
...