重构ruby on rails模型

Question

给定以下代码，

如何重构它，使方法search_word能够访问issueid？

我想说，将函数search_word更改为接受3个参数或将issueid设置为实例变量(@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

如果需要额外的上下文，可以将其作为额外的参数传递。这就是它应该如何工作的。

正如您所确定的，设置@-type实例变量来传递上下文不是一种好的形式。

有许多Ruby约定您似乎并不了解：

使用{ }.

• Use代替

• ，使用Array.new代替[ ]，使用Hash.new代替case语句或常量，而不是定义哈希，然后只检索其中一个元素，使用return丢弃{ }.

• Use，除非严格必要，因为默认情况下总是返回最后一个操作。对于多行块，

• 使用array.each do |item|而不是do ... end，而不是{ ... }，其中花括号版本通常保留为一行程序。避免与散列declarations.

• Try混淆，并避免在差异很小时复制大块代码。例如，声明一个临时变量，有条件地操作它，然后存储它，而不是定义多个独立变量。

考虑到这一点，这里是它的改写：

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