这个红宝石方法可以重构吗？

甲user具有多个libraries，并且每个库具有多个books。我想知道用户是否在他的某个库中有书。我打电话此方法：current_user.has_book?(book)：这个红宝石方法可以重构吗？

def has_book?(book) 
    retval = false 
    libraries.each do |l| 
    retval = true if l.books.include?(book) 
    end 
    return retval 
end

可我的方法进行重构？

来源

2015-08-15 Stéphane J.

为了什么目的？你最终的目标是什么？如果清晰度是你的目标，则更短不一定更清楚。 –

我的目的是通过并理解我用来生成更易维护的代码并保持符合常见用法的语言的“提示和技巧”。我不想要一个丑陋的代码，也没有10行代码，如果我能在1或2 –

在Ruby中做的工作，你总是可以用分号取代换行，所以你可以随时在1号线做的工作：'高清has_book ？（书）retval = false; libraries.each do | l |如果l.books.include？（book）结束，则retval = true;返回retval结束。不过，这并不一定会使它更易于维护。（附注：我真不明白用“更少的线”或“单行”的痴迷） –

我不知道为什么@Зелёный带走了他的答案，但是这是一个很好的IMO所以我在这里贴吧：

def has_book?(book) 
    libraries.includes(:books) 
      .where(books: {id: book.id}) 
      .any? 
end

来源

2015-08-15 11:19:22

备注：我个人尽量避免在'where'中嵌套散列，因为它们依赖于表名。另一种方法是像这样使用'merge'：'.merge（Book.where（id：book.id））''。这种改变没有任何意义，但是允许你为'Book'类的范围交换内部'where'子句。 –

这看起来是最有缩小的版本，我看不出有什么办法来缩小它更多：

def has_book?(a)c=false;libraries.each{|b|c=true if b.books.include?a};c end

来源

2015-08-15 10:16:44

def has_book?(book) 
    libraries.map { |l| l.books.include?(book) }.any? 
end

map打开库对象收集到的bool的集合，这取决于它们是否包含了书，any?回报true如果数组中的任何元素是true。

这有更少的线，但是如果你一旦你找到包含该书库返回true原来的解决方案可能是更有效的。

def has_book?(book) 
    libraries.each do |l| 
    return true if l.books.include?(book) 
    end 
    return false 
end

btw。 php和ruby都出现在同一时间。

来源

2015-08-15 11:22:09 rodic

你的第一个片段根本不需要'map'：你可以使用'collection.any？ {| element | element.whatever？ }'这和后者一样有效，因为它遇到了第一个'真'。 –

好点。感谢您纠正我。那么用任何一个块都可以解决这个问题。 – rodic

def has_book?(book) 
    libraries.any?{|lib| lib.books.include?book} 
end

非常简单，我可以想象。虽然不是零。

来源

2015-08-15 13:00:11 piezol

这个红宝石方法可以重构吗？

回答

相关问题