2009-07-13 4 views
4

私はちょうど最適であるよりもネストされていると私を殴るコードの塊を書いた。私はこのスタイルを改善する方法についてアドバイスをしたいと思います。特に、「フラットはネストよりも優れています。ネストされたTry/Exceptsのクリーンアップ

for app in apps: 
    if app.split('.', 1)[0] == 'zc': #only look for cron in zc apps 
     try: 
      a = app + '.cron' 
      __import__(a) 
      m = sys.modules[a] 

      try: 
       min = m.cron_minute() 
       for job in min: 
        k.add_interval_task(job[0], 'minute task', r(M_LB, M_UB), 
             60*job[1], 
             kronos.method.threaded,(),()) 
      except AttributeError: #no minute tasks 
       pass 

      try: 
       hour = m.cron_hour() 
       for job in hour: 
        k.add_daytime_task(job[0], 'day task', range(1, 8), None, 
             (job[1], r(H_LB, H_UB)), 
             kronos.method.threaded,(),()) 
      except AttributeError: #no hour tasks 
       pass 

     except ImportError: #no cron jobs for this module 
      pass 

編集:下からの提案を組み合わせる は、ここに私の書き換え形です。

for app in apps: 
    if app.split('.', 1)[0] != 'zc': #only look for cron in zc apps 
     continue 

    try: 
     a = app + '.cron' 
     __import__(a) 
    except ImportError: #no cron jobs for this module, continue to next one 
     continue 

    m = sys.modules[a] 
    if hasattr(m, 'cron_minute'): 
     min = m.cron_minute() 
     for job in min: 
      k.add_interval_task(job[0], 'minute task', r(M_LB, M_UB), 
           60*job[1], 
           kronos.method.threaded,(),()) 

    if hasattr(m, 'cron_hour'): 
     hour = m.cron_hour() 
     for job in hour: 
      k.add_daytime_task(job[0], 'day task', range(1, 8), None, 
           (job[1], r(H_LB, H_UB)), 
           kronos.method.threaded,(),()) 

答えて

9

主な問題は、あなたのtry節が広すぎる、特に最も外側のものです:その種の習慣では、あなたのtry/exceptの1つが突然予期しない例外を隠してしまったので、間もなく不思議なバグに遭遇しますあなたが呼んでいる他の機能から「バブルアップ」します。

だから私が代わりに、お勧めしたい:として

for app in apps: 
    if app.split('.', 1)[0] != 'zc': #only look for cron in zc apps 
     continue 

    try: 
     a = app + '.cron' 
     __import__(a) 
    except ImportError: #no cron jobs for this module 
     continue 

    # etc etc 

をさておき、私はまた、(以外/いかなる試みにも依存しない)別の方法で「フラットがネストされたよりも優れている」を適用しています」でありますループのこの脚で何もしなければならない場合は、「私は何かすることがあれば」の代わりに[すなわちループの次の脚に移動する]を続行し、その後にかなりの量の入れ子コードが続きます。 continueなどの機能を提供する言語のネストされたif(/ ifの/ continueまたはif/returnの)スタイル(Cがそれを持っているので、本質的にすべての現代スタイル)が常に好まれます。これはシンプルな「フラット対ネスト」スタイルのプリファレンスで、問題の肉は次のとおりです:あなたのtry節を小さく保ちます!最悪の場合、単にexcept節で続行したり返すことができない場合は、try/except/else:try節に置くことができます。これは絶対に必要なものだけです。 raise - else節に次のコードの残りの部分(想定されていない部分または発生する部分)を入れます。これはネストを変更するものではありませんが、期待していない例外を誤って隠すリスクを大幅に削減できます!

+0

ありがとう、これは私が必要としていたものです。私はこの方法でcontinueを使うことを忘れていました。 できるだけ小さくしようとすることについてのアドバイスは意味があります。継続を使用すると、私はそれを行うことができます。書き直された形でははるかに明示的です。 –

0

あなたがメインのロジックを実行する関数、およびステートメントのtry ... exceptでそれを包む、その関数を呼び出す別の関数を作成することができます。次に、メインアプリケーションでは、すでに例外を処理している関数を呼び出すことができます。 (「クリーンコード」ブックの推奨事項に基づく)。

+0

このコードは1回だけ呼び出されます。実際にレイアウトを改善するのではなく、実際にスパゲッティコードを生成するだけです。 –

0

ここでのトリックは、それらが壊れているかどうかを調べることです。これは、の処理です。部分は、例外処理です。少なくとも、コメントの前提を述べる警告を出すこと。あなたが自分より先に進んでいるように実際の処理の前に過度のネスティングを心配している。スタイリッシュになる前に心配しています。

+0

あなたはコードが何をしているのか誤解しています。例外が検出された場合は何もしないことになっています。チェックされているものが存在しない場合はエラーではありません(実際はほとんどの場合そうなりません)。私はオプションの関数のすべてのインスタンスが見つからないために警告が表示されないようにしたいと思います。 –

1

時間単位ごとのジョブが実際に壊れている場合は、AttibuteErrorやその他の例外が発生するのだろうか?特に、本当に逮捕された仕事について何かがある場合は、おそらくそれらをキャッチする必要はありません。

もう1つのオプションは、例外ハンドラを可能な限り例外に近づけるように、問題のコードをtry-catchでラップすることです。ここにはスタブがあります:

for app in apps: 
    if app.split('.', 1)[0] == 'zc': #only look for cron in zc apps 
     try: 
      a = app + '.cron' 
      __import__(a) 
      m = sys.modules[a] 
     except ImportError: #no cron jobs for this module 
          #exception is silently ignored 
          #since no jobs is not an error 
      continue 
     if hasattr(m, "cron_minute"): 
      min = m.cron_minute() 
      for job in min: 
       k.add_interval_task(job[0], 'minute task', r(M_LB, M_UB), 
            60*job[1], 
            kronos.method.threaded,(),()) 

     if hasattr(m, "cron_hour"): 
      hour = m.cron_hour() 
      for job in hour: 
       k.add_daytime_task(job[0], 'day task', range(1, 8), None, 
            (job[1], r(H_LB, H_UB)), 
            kronos.method.threaded,(),()) 

通知ここでは例外ハンドラは1つしかありませんが、正しく無視することで処理します。 私たちは一つの属性か他の属性が存在しない可能性を予測できるので、コードを明快にするのに役立ちます。 これを明示的にチェックしてください。それ以外の場合は、 が本当に明白ではなく、なぜあなたはAttributeErrorをキャッチしているのですか、それともそれを上げているのでしょうか。

+0

ありがとう!ここでhasattr()を使うのは、try/exceptを使うよりはるかにクリーンなアプローチです。 –

関連する問題