Рефакторинг нескольких операторов if для аутентификации пользователя с поддоменами

Question

Я создаю типичное веб-приложение, в котором, когда пользователь регистрируется, он получает доступ к приложению через свой собственный поддомен (company.myapp.com).Часть «Проверка, какой пользователь вошел в систему», начинает становиться очень волосатой, и ее, очевидно, нужно хорошо написать, потому что она запускается так часто, поэтому мне было интересно, как вы, ребята, перефакторили бы этот материал.

Вот различные состояния:

1. Пользователь должен войти в систему, у пользователя не должно быть названия компании, а поддомен должен быть пустым
2. Aпользователь должен войти в систему, пользователь должен иметь название компании, это имя компании должно соответствовать текущему субдомену
3. пользователь должен войти в систему, пользователь должен иметь название компании, это имя компании должно совпадатьтекущий поддомен и логическое значение is_admin пользователя - true

if !session[:user_id].nil?
  @user = User.find(session[:user_id])
  if @user.company.nil? && request.subdomains.first.nil?
    return "state1"
  elsif !@user.company.nil?
    if @user.company.downcase == request.subdomains.first.downcase && !@user.is_admin
      return "state2"
    elsif @user.company.downcase == request.subdomains.first.downcase && @user.is_admin
      return "state3"
    end
  end
end

Answer 1

Как сказал Эндрю, вам нужно иметь эту логику в вашей пользовательской модели. Я сделаю еще один шаг и сделаю каждое условие методом, обеспечивающим чистоту, понятность, возможность повторного использования кода и соблюдение принципа единой ответственности. А также определите метод в пользовательской модели, который возвращает состояние, например, так.

class User

  def get_state(request)
    return 'state1' if no_company_and_subdomain?(request)
    return 'state2' if has_company_that_match_subdomain?(request)
    return 'state3' if admin_has_company_that_match_subdomain?(request)
  end

  def no_company_and_subdomain?(request)
    company.nil? && request.subdomains.empty?
  end

  def has_company_that_match_subdomain?(request)
    return false if company.nil? || is_admin
    company.downcase == request.subdomains.first.downcase
  end

  def admin_has_company_that_match_subdomain?(request)
    return false if company.nil? || !is_admin
    company.downcase == request.subdomains.first.downcase
  end

end

и тогда ваш код контроллера станет таким

if !session[:user_id].nil?
  @user = User.find(session[:user_id])
  @user.get_state
end

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

Answer 2

Проверьте замените тип полиморфизмом и замените тип стратегией шаблон рефакторинга (в вашем случае последний подходит лучше) Это может упростить ваш код, особенно если в вашем коде несколько похожих условий.

Рефакторинг Ruby является хорошим источником информации по этой теме.

Answer 3

Можете ли вы перенести некоторую логику в модель пользователя, вместо того, чтобы использовать ее в контроллере? Таким образом, ваш код будет более СУХИМ и лучше инкапсулирован.