如何使這個更面向對象？

Question

我是一個鐵軌小老闆，試圖追隨那些在我之前來過的人的DRY方法。 我知道我做錯了什麼 - 我只是不確定那是什麼或如何克服它。

基本上，我的問題是如何讓這段代碼更面向對象？

我有一個類Podcast，它在這一點上只包含一堆類方法，它們可以從網上抓取各種數據。

因此，舉例來說，這類方法試圖從他們的網站上發現了一個播客Twitter或Facebook飼料：

def self.social_discovery(options = {}) 
    new_podcasts_only = options[:new_podcasts_only] || false 
    if new_podcasts_only 
    podcast = Podcast.find(:all, :select => 'id, siteurl, name', :conditions => ['created_at > ? and siteurl IS NOT ?', Time.now - 24.hours, nil]) 
    Podcast.podcast_logger.info("#{podcast.count}") 
    else 
    podcast = Podcast.find(:all, :select => 'id, siteurl, name', :conditions => ['siteurl IS NOT ?', nil]) 
    end 

    podcast.each do | pod | 
    puts "#{pod.name}" 
    begin 
     # Make sure the url is readable by open-uri 
     if pod.siteurl.include? 'http://' 
     pod_site = pod.siteurl 
     else 
     pod_site = pod.siteurl.insert 0, "http://" 
     end 

     # Skip all of this if we're dealing with a feed 
     unless pod_site.downcase =~ /.rss|.xml|libsyn/i 
     pod_doc = Nokogiri.HTML(open(pod_site)) 
     pod_name_fragment = pod.name.split(" ")[0].to_s 
     if pod_name_fragment.downcase == "the" 
      pod_name_fragment = pod.name.split(" ")[1].to_s unless pod.name.split(" ")[1].to_s.nil? 
     end 
     doc_links = pod_doc.css('a') 

     # If a social url contains part of the podcast name, grab that 
     # If not, grab the first one you find within our conditions 
     # Give Nokogiri some room to breathe with pessimistic exception handling 
     begin 
      begin   
      twitter_url = doc_links.find {|link| link['href'] =~ /twitter.com\// and link['href'].match(/#{pod_name_fragment}/i).to_s != "" unless link['href'] =~ /share|status/i}.attribute('href').to_s 
      rescue Exception => ex 
      if doc_links.find {|link| link['href'] =~ /twitter.com\// unless link['href'] =~ /share|status/i}.nil? 
       twitter_url = nil 
      else  
       twitter_url = doc_links.find {|link| link['href'] =~ /twitter.com\// unless link['href'] =~ /share|status/i}.attribute('href').to_s 
      end 
      end 

      begin  
      facebook_url = doc_links.find {|link| link['href'] =~ /facebook.com\// and link['href'].match(/#{pod_name_fragment}/i).to_s != "" unless link['href'] =~ /share|.event/i}.attribute('href').to_s 
      rescue Exception => ex 
      if doc_links.find {|link| link['href'] =~ /facebook.com\// unless link['href'] =~ /share|.event/i}.nil? 
       facebook_url = nil 
      else  
       facebook_url = doc_links.find {|link| link['href'] =~ /facebook.com\// unless link['href'] =~ /share|.event/i}.attribute('href').to_s 
      end 
      end 
     rescue Exception => ex 
      puts "ANTISOCIAL" 
     # Ensure that the urls gets saved regardless of what else happens 
     ensure 
      pod.update_attributes(:twitter => twitter_url, :facebook => facebook_url)    
     end 

     puts "#{twitter_url}" + "#{facebook_url}" 
     Podcast.podcast_logger.info("#{twitter_url}" + "#{facebook_url}") 
     end 
    rescue Exception => ex 
     puts "FINAL EXCEPTION: #{ex.class} + #{ex.message}" 
    end 
    end 
end 

同樣，我知道這是不好的代碼。請幫我理解爲什麼？ 我將永遠在你的債務。

感謝，

哈里斯

Answer 1

我可以在你的代碼中看到的主要事情是代碼重複。如果仔細觀察，獲取twitter url的代碼和facebook url的代碼幾乎完全相同，除了'twitter.com'和'facebook.com'部分。我的建議是將它解釋爲一個單獨的方法，它將doc_links變量作爲參數以及查找鏈接的正則表達式。此外，我不明白爲什麼你在這裏做「除非......」部分：

if pod_name_fragment.downcase == "the" 
    pod_name_fragment = pod.name.split(" ")[1].to_s unless pod.name.split(" ")[1].to_s.nil? 
end 

如果你不這樣做該行的「除非......」的一部分，pod_name_fragment會被定義但nil，但如果你不包含它，你會得到一個例外，如果你試圖參考pod_name_fragment。

此外，你應該幾乎不會拯救Exception。改爲使用StandardError。假設你的程序正在運行，你試着用Ctrl-C取消它。這拋出了一個SystemExit（我不是100％確定名稱）異常，它是異常出口的子類。在大多數情況下，您會立即退出。我知道這對於一個Rails應用程序來說並不是那麼適用，但我確信有其他原因可以捕獲SystemError。

可能還有更多，一個簡單的方法來查找「錯誤的代碼」是看指標。 Ryan Bates在度量指標（http://railscasts.com/episodes/252-metrics-metrics-metrics）上做出了極好的Railscast，並且我建議你看看Reek特別是尋找「代碼味道」。查看他們的wiki瞭解不同事物的含義。