這個紅寶石方法可以重構嗎？

甲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

這個紅寶石方法可以重構嗎？

回答

相關問題