0

私はレール初心者で、先人たちの DRY メソッドに従おうとしています。私は自分が何か間違ったことをしていることを知っています – それが何であるか、またはそれを克服する方法がわかりません.

基本的に、私の質問は、このコードをよりオブジェクト指向にするにはどうすればよいかということです。

Podcast クラスがあります。この時点では、Web からさまざまなデータをスクレイピングする一連のクラス メソッドが含まれています。

たとえば、次のクラス メソッドは、ポッドキャストの twitter または facebook フィードを Web サイトから検出しようとします。

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.com」と「facebook.com」の部分を除いて、twitterurlとfacebookurlのコードはほぼ同じです。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

行の「unless...」の部分を実行しない場合、pod_name_fragmentは定義されますがnil、これを含めない場合、を参照しようとすると例外が発生しますpod_name_fragment。

また、ほとんど救助するべきではありませんException。StandardError代わりに使用してください。プログラムが実行中で、Ctrl-Cでキャンセルしようとしたとします。SystemExitこれにより、例外出口のサブクラスである（名前について100％確信が持てない）例外がスローされます。ほとんどの場合、すぐに終了する必要があります。これはRailsアプリにはあまり当てはまらないことは知っていますが、代わりにSystemErrorをキャッチする理由は他にもあると確信しています。

「悪いコード」を見つける簡単な方法の1つは、メトリックを調べることです。Ryan Batesは、メトリクス（ http://railscasts.com/episodes/252-metrics-metrics-metrics ）で優れたRailscastを作成しました。特に、「コードの臭い」を見つけるためにReekを調べることをお勧めします。さまざまな意味については、彼らのwikiを確認してください。

于 2011-02-15T02:22:03.427 に答える