0

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

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

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

因此，例如，这个类方法试图从他们的网站上发现一个 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

同样，我知道这是错误的代码。请帮我理解为什么？我将永远欠你的债。

谢谢，

哈里斯

4

1 回答 1

2

我在您的代码中可以看到的主要内容是代码重复。如果仔细观察，获取 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 but nil，但如果您不包含它，如果您尝试引用pod_name_fragment.

Also, you should almost never rescue Exception. Use StandardError instead. Let's say your program is running and you try to cancel it with Ctrl-C. That throws a SystemExit (I'm not 100% sure about the name) exception, which is a subclass of the Exception exit. In most cases you would then want to exit right away. I know this isn't that much applicable for a Rails app, but I'm pretty sure there's other reasons to catch SystemError instead.

There might be more, one easy way to find "bad code" is to look at metrics. Ryan Bates made an excellent Railscast on metrics (http://railscasts.com/episodes/252-metrics-metrics-metrics), and I'd suggest you look at Reek in particular for finding "code smells". Check their wiki for information on what the different things mean.

于 2011-02-15T02:22:03.427 回答