2017-09-08 4 views
1

私はこのコードをネストされたif elseのケースで書いていましたが、それを改善するための方法があれば、それはとても醜いと思います。複数のifを入れ子にしてコードを埋め込む方法はありますか?

def do_something(self, response): 
    a_url = response.css('a.classA::attr(href)').extract_first() 
    if a_url: 
     a_url = a_url.split('&')[0] 
    else: 
     a_url = response.css('a.classB::attr(href)').extract_first() 
     if a_url: 
      a_url = a_url.split('&')[0] 
     else: 
      logger.error('get no url') 
    if a_url: 
     yield Request(
      url=a_url, 
      dont_filter=True, 
      callback=self.do_next_thing 
     ) 

主な懸念は、私はレスポンスからURL /リンクを抽出し、その後、分割それと最初の要素を取得したいということです。しかし、a_urlは、2つの(またはそれ以上の)要素のうちの1つにしか存在しません。 分割を直接行うことはできません。a_urlNoneTypeとなる可能性があります。私はtry except elseで試してみたいが、それはさらに複雑になっているようだ。

もっと良い解決法はありますか?

+2

https://codereview.stackexchange.com/ –

答えて

4

私はあなたがこのようにそれを行うことができると思う:

def do_something(self, response): 
    a_url = (
     response.css('a.classA::attr(href)').extract_first() 
     or 
     response.css('a.classB::attr(href)').extract_first() 
    ) 

    if not a_url: 
     logger.error('get no url') 
     return # or raise an exception and let the caller do the logging 

    yield Request(
     url=a_url.split('&')[0], 
     dont_filter=True, 
     callback=self.do_next_thing 
    ) 

これは短いを使用していますor operatorの-circuit行動:

表現xまたはyは最初にxを評価します。 xが真の場合は、 の値が返されます。そうでない場合はyが評価され、結果の値は になります。また、「早期復帰」技術を使用し

、即ち故障の場合は、最初に処理され、その後、「通常」の場合は、外部任意if又はelseを行うことができます。

2

このコードを単純化するための最良の方法は、最初の場所でけんか腰で両方のクラスを選択することです:

def do_something(self, response): 
    a_url = response.css("a.classA::attr(href), a.classB::attr(href)") 
    if a_url: 
     yield Request(
      url=a_url.split('&')[0], 
      dont_filter=True, 
      callback=self.do_next_thing 
     ) 
    else: 
     logger.error('get no url') 
0

メソッドを2つ(後で3つ)に分割することを検討してください。私が見ているように、最初の行は実際の論理よりも準備が多いからです。このような何か:

def prepare_something(self, response): 
    a_url = response.css('a.classA::attr(href)').extract_first() 
    if a_url: 
    return a_url.split('&')[0] 
    else: 
    a_url = response.css('a.classB::attr(href)').extract_first() 
    if a_url: 
     return a_url.split('&')[0] 
    else: 
     logger.error('get no url') 
     return None 


def do_something(self, response): 
    a_url = self.prepare_something(response) 
    if a_url: 
    yield Request(
     url=a_url, 
     dont_filter=True, 
     callback=self.do_next_thing 
    ) 

この方法では、私見、コードが少しクリーナーで、あなたは以下のように、prepare_something方法をリファクタリングする場合がありますことを確認することができます:

def get_a_url_part(self, response, path): 
    a_url = response.css(path).extract_first() 
    return a_url.split('&')[0] if a_url else None 

def prepare_something(self, response): 
    a_url = self.get_a_url_part(response, 'a.classA::attr(href)') 
    b_url = self.get_a_url_part(response, 'a.classB::attr(href)') 
    return a_url if a_url else b_url 

def do_something(self, response): 
    a_url = self.prepare_something(response) 
    if a_url: 
    yield Request(
     url=a_url, 
     dont_filter=True, 
     callback=self.do_next_thing 
    ) 

私の視点からは、これは改善と見なすことができます。

よろしくお願いいたします。

関連する問題