Совет по рефакторингу - Ruby 101 - использование нескольких вложенных if-then - PullRequest
0 голосов
/ 23 ноября 2011

Какова предписанная лучшая практика для рефакторинга следующего?

def has_thing?
  if things and things[:something] and things[:something][:else] and things[:something][:else][:matters]
    return true
  else
    return false
  end
end

Я в основном хочу вернуть true, если things[:something][:else][:matters] не ноль, но return true if things[:something][:else][:matters] завершается неудачно, когда things ноль или ... things[:something] ноль с, например, undefined method '[]' for nil:NilClass.

Я мог бы обернуть все это в begin...rescue..end и вернуть false, если было сгенерировано исключение, но это тоже не кажется правильным!

Ответы [ 5 ]

4 голосов
/ 23 ноября 2011

Можно предложить два варианта:

def has_thing?
  !!(things && things[:something] && things[:something][:else] && things[:something][:else][:matters])
end

Или для более удобочитаемого

def has_thing?
  return false unless things
  return false unless things[:something]
  return false unless things[:something][:else]
  return false unless things[:something][:else][:matters]
  true
end
2 голосов
/ 23 ноября 2011

Обратите внимание, что этот код:

if something
  return true
else
  return false
end

полностью эквивалентно:

!!something

Или напрямую something, если это логическое значение. Также обратите внимание, что нет смысла писать return явно при написании последнего выражения метода.

Также старайтесь избегать повторения проверок в хэше. Я бы порекомендовал ick возможно .

Кроме того, в вашем * core_ext * модуле вы можете использовать этот метод:

class Object
  def to_bool
    !!self
  end
end

В конце код выглядит намного более кратким и декларативным:

def has_thing?
  things[:something].maybe[:else].maybe[:matters].to_bool
end
1 голос
/ 24 ноября 2011

Я бы, наверное, согласился с:

def has_thing?
  !!(
    things &&
    things[:something] &&
    things[:something][:else] &&
    things[:something][:else][:matters]
  )
end

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

Вертикальное выравнивание облегчает намбыстро сканировать изменяющиеся строки и видеть, что нового.

1 голос
/ 23 ноября 2011

Вы можете использовать try:

def has_thing?
  !! things.try(:[], :something).try(:[], :else).try(:[], :matters)
end

Более подробную версию, используя Hash#fetch:

def has_thing?
  true if things &&
          things.fetch(:something){ return false }.
                 fetch(:else)     { return false }.
                 [:matters]
end
0 голосов
/ 23 ноября 2011

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

def has_things?
  things.andand[:something].andand[:else].andand[:matters]
end

Хотя это не очень красиво, это решит проблему.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...