Рефакторинг рубина на рельсах модели

Question

Учитывая следующий код,

Как бы вы изменили рефакторинг, чтобы у метода search_word был доступ к Issidid?

Я бы сказал, что изменение функции search_word таким образом, чтобы она принимала 3 аргумента, или создание значения переменной экземпляра (@issueid), можно рассматривать как пример плохой практики, но, честно говоря, я не могу найти никакого другого решения. Если кроме этого нет решения, не могли бы вы объяснить причину, почему нет другого решения?

Имейте в виду, что это модель Ruby on Rails.

def search_type_of_relation_in_text(issueid, type_of_causality)
    relation_ocurrences = Array.new
    keywords_list = { 
        :C => ['cause', 'causes'],
        :I => ['prevent', 'inhibitors'],
        :P => ['type','supersets'],
        :E => ['effect', 'effects'],
        :R => ['reduce', 'inhibited'],
        :S => ['example', 'subsets'] 
    }[type_of_causality.to_sym]  

    for keyword in keywords_list
        relation_ocurrences + search_word(keyword, relation_type)
    end        

    return relation_ocurrences
end


def search_word(keyword, relation_type)
relation_ocurrences = Array.new

@buffer.search('//p[text()*= "'+keyword+'"]/a').each { |relation|

    relation_suggestion_url   = 'http://en.wikipedia.org'+relation.attributes['href']
    relation_suggestion_title = URI.unescape(relation.attributes['href'].gsub("_" , " ").gsub(/[\w\W]*\/wiki\//, ""))

    if not @current_suggested[relation_type].include?(relation_suggestion_url)
        if @accepted[relation_type].include?(relation_suggestion_url)
            relation_ocurrences << {:title => relation_suggestion_title, :wiki_url => relation_suggestion_url, :causality => type_of_causality, :status => "A", :issue_id => issueid}
        else
            relation_ocurrences << {:title => relation_suggestion_title, :wiki_url => relation_suggestion_url, :causality => type_of_causality, :status => "N", :issue_id => issueid}
        end

    end
} 

end

Answer 1

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

Задание переменных экземпляра типа @ для передачи контекста - плохая форма, как вы определили.

Существует ряд соглашений Ruby, о которых вы, похоже, не подозреваете:

• Вместо Array.new просто используйте [ ], а вместо Hash.new используйте { }.
• Используйте оператор case или константу вместо определения хэша, а затем извлечения только одного из элементов, отбрасывая остаток.
• Избегайте использования return без крайней необходимости, так как последняя операция всегда возвращается по умолчанию.
• Используйте array.each do |item| вместо for item in array
• Используйте do ... end вместо { ... } для многострочных блоков, где версия с фигурными скобками обычно зарезервирована для однострочников. Избегает путаницы с объявлениями хешей.
• Старайтесь избегать дублирования больших кусков кода, когда различия незначительны. Например, объявите временную переменную, условно управляйте ею, а затем сохраните ее вместо определения нескольких независимых переменных.

Имея это в виду, вот переделка:

KEYWORDS = { 
    :C => ['cause', 'causes'],
    :I => ['prevent', 'inhibitors'],
    :P => ['type','supersets'],
    :E => ['effect', 'effects'],
    :R => ['reduce', 'inhibited'],
    :S => ['example', 'subsets'] 
}

def search_type_of_relation_in_text(issue_id, type_of_causality)
  KEYWORDS[type_of_causality.to_sym].collect do |keyword|
    search_word(keyword, relation_type, issue_id)
  end
end

def search_word(keyword, relation_type, issue_id)
  relation_occurrences = [ ]

  @buffer.search(%Q{//p[text()*= "#{keyword}'"]/a}).each do |relation|
    relation_suggestion_url = "http://en.wikipedia.org#{relation.attributes['href']}"
    relation_suggestion_title = URI.unescape(relation.attributes['href'].gsub("_" , " ").gsub(/[\w\W]*\/wiki\//, ""))

    if (!@current_suggested[relation_type].include?(relation_suggestion_url))
      occurrence = {
        :title => relation_suggestion_title,
        :wiki_url => relation_suggestion_url,
        :causality => type_of_causality,
        :issue_id => issue_id
      }

      occurrence[:status] =
        if (@accepted[relation_type].include?(relation_suggestion_url))
          'A'
        else
          'N'
        end

      relation_ocurrences << occurrence
    end
  end 

  relation_occurrences
end