我怎样才能使它更加面向对象？

Question

我是一个尝试遵循前人的 DRY 方法的 Rails 菜鸟。 我知道我做错了什么——我只是不确定那是什么或如何克服它。

基本上，我的问题是如何让这段代码更面向对象？

我有一个 Podcast 类，此时它只包含一堆从网络上抓取各种数据的类方法。

因此，例如，这个类方法试图从他们的网站上发现一个 podcast twitter 或 facebook feed：

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，但如果你不包含它，如果你尝试引用@，你会得到一个异常987654326@.

另外，你几乎不应该拯救Exception。请改用StandardError。假设您的程序正在运行，并且您尝试使用 Ctrl-C 取消它。这会引发SystemExit（我不是 100% 确定名称）异常，它是 Exception 出口的子类。在大多数情况下，您会想要立即退出。我知道这不太适用于 Rails 应用程序，但我很确定还有其他原因可以捕获 SystemError。

可能还有更多查找“不良代码”的简单方法是查看指标。 Ryan Bates 在指标方面做了出色的 Railscast (http://railscasts.com/episodes/252-metrics-metrics-metrics)，我建议您特别关注 Reek 以发现“代码气味”。查看他们的wiki，了解不同事物的含义。