Как упростить большие условия

Question

У меня есть пять выпадающих меню, и мне нужно поставить условия для каждого из них.Мой код:

   def search(search, compare, year, rain_fall_type)
    if search == 'All'
      if rain_fall_type == 'All'
        all
      else
        if year == 'All'
          if rain_fall_type == "None"
            where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id')
          else
            # all
             where(Sector: rain_fall_type).order('id')
          end
        else
          if rain_fall_type == "All"
            order("#{year} ")

          elsif rain_fall_type == "None"
            where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id')
           else
            where(Sector: rain_fall_type).order("#{year} ")
           end
        end
        # where(Year: year).order("#{rain_fall_type} ")
      end
    elsif compare != "None"
      if year == 'All'
        where('Sector = ? OR Sector = ?', rain_fall_type, compare).order(:id)
      else
        where('Sector = ? OR Sector = ?', rain_fall_type, compare).order(:id)
      end
    else

      if rain_fall_type == 'All'
        all.order('id')
      else
        if year == 'All'

          if rain_fall_type == "None"
            where('Sector = ? ', search).order('id')
          else

            where('Sector = ? ', rain_fall_type).order('id')
          end
        else

          if rain_fall_type == "None"

            if search == "All"
              where('Sector = ? ', search).order('id')
            else
              where('Sector = ? ', search).order('id')
            end
          else
            # all
            where('Sector = ? ', rain_fall_type).order('id')
          end
        end
      end
    end
  end

Он имеет много if и else.Я пытаюсь минимизировать условия.Что может быть лучшим способом уменьшить этот код?Кто-то предложил мне вместо этого использовать switch case.Должен ли я использовать это?Если да, то как?

Answer 1

Это, вероятно, лучшее объяснение того, как вы должны настроить это.

class Product < ActiveRecord::Base
  # custom_scope_1
  scope :status, -> (status) { where status: status }
  # custom_scope_2
  scope :location, -> (location_id) { where location_id: location_id }
  # custom_scope_3
  scope :search, -> (name) { where("name like ?", "#{name}%")}
end

def index
   @products = Product.where(nil) # creates an anonymous scope
   @products = @products.status(params[:status]) if params[:status].present?
   @products = @products.location(params[:location]) if params[:location].present?
   @products = @products.search(params[:search]) if params[:search].present?
end

Это может быть очищено дальше ...

def index
  @products = Product.where(nil)
  filtering_params(params).each do |key, value|
    @products = @products.public_send(key, value) if value.present?
  end
end

private

# A list of the param names that can be used for filtering the Products
def filtering_params(params)
  params.slice(:status, :location, :search)
end

Этот метод использует метаданные ruby-программирование для циклического перебора параметров и динамического вызова предопределенных областей для модели

Вы можете переместить этот код в модуль и включить его в любую модель, поддерживающую фильтрацию

app/models/concerns/filterable.rb

module Filterable
  extend ActiveSupport::Concern

  module ClassMethods
    def filter(filtering_params)
      results = self.where(nil)
      filtering_params.each do |key, value|
        results = results.public_send(key, value) if value.present?
      end
      results
    end
  end
end

app/models/product.rb

class Product
  include Filterable
  ...
end

app/controllers/product_controller.rb

def index
  @products = Product.filter(params.slice(:status, :location, :search))
end

Теперь у вас есть фильтрация и поиск ваших моделей с одной линией в контроллереи одна строчка в модели

Answer 2

Прежде всего, часть вашей логики не имеет смысла:

def search(search, compare, year, rain_fall_type)
  if search == 'All'
    if rain_fall_type == 'All'
      all
    else
      # rain_fall_type != 'All'
      if year == 'All'
        if rain_fall_type == "None"
          where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id')
        else
          where(Sector: rain_fall_type).order('id')
        end
      else
        # in rain_fall_type != 'All' branch, so meaningless 'if'
        if rain_fall_type == "All"
          order("#{year} ")
        elsif rain_fall_type == "None"
          where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id')
        else
          where(Sector: rain_fall_type).order("#{year} ")
        end
      end
    end
  elsif compare != "None"
    # both are same, so meaningless 'if'
    if year == 'All'
      where('Sector = ? OR Sector = ?', rain_fall_type, compare).order(:id)
    else
      where('Sector = ? OR Sector = ?', rain_fall_type, compare).order(:id)
    end
  else
    # search != 'All'
    if rain_fall_type == 'All'
      all.order('id')
    else
      if year == 'All'
        if rain_fall_type == "None"
          where('Sector = ? ', search).order('id')
        else
          where('Sector = ? ', rain_fall_type).order('id')
        end
      else
        if rain_fall_type == "None"
          # in search != 'All' branch, so meaningless 'if'
          # AND both are same, so again meaningless 'if'
          if search == "All"
            where('Sector = ? ', search).order('id')
          else
            where('Sector = ? ', search).order('id')
          end
        else
          where('Sector = ? ', rain_fall_type).order('id')
        end
      end
    end
  end
end

Это еще не все, и я не буду указывать на это все, потому что мы все равно выкидываем все эти if вещи.

В конечном счете, мы собираемся отложить запрос до конца метода, например:

def search(search, compare, year, rain_fall_type)

  ...

  @query = all 
  @query = @query.where(Sector: @sectors) if @sectors
  @query = @query.order(@order) if @order
  @query

end

Таким образом, вы берете все ваших where и order утверждений и делаете их только один раз в конце. Это экономит много печатать прямо там. Смотрите комментарий от muistooshort о том, почему (Sector: @sectors) работает.

Итак, трюк заключается в установке @sectors и @order. Во-первых, я собираюсь назначить входные переменные переменным экземпляра, потому что мне это нравится (и чтобы избежать путаницы между переменной @search и методом search):

def search(search, compare, year, rain_fall_type)
  @search, @compare, @year, @rain_fall_type = search, compare, year, rain_fall_type

  ...

  @query = all 
  @query = @query.where(Sector: @sectors) if @sectors
  @query = @query.order(@order) if @order
  @query
end

Так вот, этот ответ продолжается слишком долго, поэтому я не буду тащить вас через все подробности. Но, добавив пару вспомогательных методов (sectors_to_use и order_to_use) и подставив их вместо @sectors и @order, вы в итоге получите следующее:

def search(search, compare, year, rain_fall_type)
  @search, @compare, @year, @rain_fall_type = search, compare, year, rain_fall_type
  @query = all 
  @query = @query.where(Sector: sectors_to_use) if sectors_to_use
  @query = @query.order(order_to_use) if order_to_use
  @query
end

private 

def sectors_to_use
  return [@rain_fall_type, @compare] if @search != 'All' && @compare != 'None'
  unless @rain_fall_type == 'All'
    if @rain_fall_type == 'None'
      @search == 'All' ? ['Primary', 'Secondary', 'Tertiary'] : [@search]
    else
      [@rain_fall_type]
    end  
  end
end

def order_to_use
  return nil if (@search == 'All') && (@rain_fall_type == 'All')
  return @year if (@search == 'All') && !(@year == 'All')
  return :id
end

Это меньше половины строк кода, более тысячи символов и намного меньше ifs.

Answer 3

Вы можете использовать защитное заявление, которое в основном return something if some_condition?. Это возможно только в определенных сценариях (где одно из условий выполняет один оператор:

Плохой пример:

if condition?
  do_something
else
  do_something_else
end

Это может быть записано как:

return do_something if condition?
do_something_else

Это даст вам меньше разветвлений кода.

---

Кроме того, другая рекомендация - вызывать другой метод с большим количеством условий вместо условий вложенности в одном кадре.

Плохой пример:

if condition?
  if condition_two?
    do_something_two
  else
    do_something
  end
else
  do_something_else
end

Это может быть записано как:

if condition?
  call_another_method 
else
  do_something_else
end

def call_another_method
  if condition_two?
    do_something_two
  else
    do_something
  end
end

---

Примером из вашего кода может быть:

if rain_fall_type == 'All'
  all
else
  if year == 'All'
    if rain_fall_type == "None"
      where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id')
    else
      # all
        where(Sector: rain_fall_type).order('id')
    end
  else
    if rain_fall_type == "All"
      order("#{year} ")

    elsif rain_fall_type == "None"
      where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id')
      else
      where(Sector: rain_fall_type).order("#{year} ")
      end
  end
end

Это можно преобразовать в:

return all if rain_fall_type == 'All'
if year == 'All'
  return where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id') if rain_fall_type == "None"
  where(Sector: rain_fall_type).order('id')
else
  return order("#{year} ") if rain_fall_type == "All"
  return where('Sector = ? OR Sector = ? OR Sector = ?', "Primary", 'Secondary', 'Tertiary').order('id') if rain_fall_type == "None"
  where(Sector: rain_fall_type).order("#{year} ")
end

Надеюсь, это может помочь:)

ПРИМЕЧАНИЕ: Это ответ на оригинальный вопрос How to simplify big conditions?. Но оригинальный пост не использует Rails / Ruby для поиска и фильтрации, а также не использует области видимости.