2012-03-16 11 views
2

混乱するタイトルについては残念です。関数の先頭に「早く戻る」のが良いスタイル(またはより効率的)ですか?

私は最近、自分のプロジェクトでこれをやり始めました。それがより効率的かどうか、そしてそれが練習するのはひどいスタイルであるかどうか疑問に思っています。ここで

は、データベースインタフェースからの例です:

def register(self, user, pw): 
    """Register user/pw into the database""" 
    if self.isStarted(): 
     raise Exceptions.Started 
    hashed = hashlib.sha512(pw).hexdigest() 
    self._db_cur.execute('''INSERT INTO PLAYERS (name, password) 
            values (?, ?)''', [user, hashed]) 
    self._db.commit() 

私は例外を発生させることで、ここでそれを行うが、私はリターンと他の場所でそれをやりました。

これは、関数を続けて実行する代わりに、偽のケースが上部の関数を終了することを可能にし、実行するコードがもうないかどうかを確認します。

私はこれをコードで見ることはほとんどありません。これは悪い習慣ですか、それとも私が想像しているようなパフォーマンスはありませんか?明確に支援するため

、どのような私はに慣れていることである:

if (somethingTrue): 
    runThis() 
    thisToo() 
    x = andThis() 
    return x 
return None 

と私は何をし始め、そして上あやふや午前ました:

if (!somethingTrue): 
    return None 
runThis() 
thisToo() 
x = andThis() 
return x 

後者は与えているようですそのように意図されたときに、コードが条件の一部ではない印象(特に4行以上の関数で)。これはまたPEP-8を遵守しているようにも見えるようになりましたので、私は本当にそれについて話しています。

私はこれが恐ろしく神聖なものを壊すと感じています。これは大丈夫ですか、または礼儀正しくありませんか?

+2

「効率性」や「パフォーマンス」を心配する必要はありません。*どちらも得られない*。あなたとあなたのチームが一番好きなように書く。一番のことは、あなたのパターンで一貫していることです*。 –

+0

私は、特に体の内部が長い場合、私は2番目のフォーム(早期のエラーチェックとリターン)を好む傾向があることを認めなければなりません。余分なインデントを追跡することは、心配するもう一つのことであり、コードを読むことは既にそれほど難しいです。 –

答えて

4

質問の一部は、スタイルとベストプラクティスに関するものです。したがって、IMHOは正しい方法ではありません。

私の意見では、ネストされたバージョン(直接リターンはありません)は、 'C'のような '歴史的な'プログラミング言語から来ており、全体のクリーンアップが適切な場所で一度行われます。その後、リソースリークがあるかもしれないので - 最後の1以外のラインから復帰することは正しくないです。ここ

int f() { 
    int result = 1; 
    char * buffer = (char *)malloc(77); 
    if(buffer!=NULL) { 
     int const fd = open("/tmp/data.log", O_RDONLY); 
     if(fd!=-1) { 
     ssize_t const read_cnt = read(fd, buffer, 77); 
     if(read_cnt!=77) { 
      /* Do something: was not possible to read 77 bytes. */ 
      result = 0; 
     } 
     close(fd); 
     } 
     free(buffer); 
    } 
    return result; 
} 

:人工の例では、唯一のポイントを表示します。

デストラクタで完全に破棄されたオブジェクトのみを使用する場合、またはリソースをクリーンアップする必要がない場合(割り当てられていないため)、私は「短い」パスの返りを優先します。これは、関数の前提条件が満たされていない場合、関数本体を '本当に'実行する方法がないように、物事をより明確にします。また、あなたはあまりインデントする必要はなく、読むのが簡単です。

パフォーマンス:いくつかのテストを行いました。私は2つの方法の違いを測定することができませんでした。 IMHOこのレベルでパフォーマンスを調整する必要がある場合は、別のプログラミング言語を選択することを考えてください。 ;-)

+0

私はいつももっとストイックなやり方でやっているのだろうと思っています。ありがとう! パフォーマンステストをしていただきありがとうございます。私はどこで試してみるか分かりませんでした。間違いなくコードをきれいにする!そして、私は必ずしもコードからその多くのマイクロパフォーマンスを絞る必要はありませんが、私はこれを学習触媒として使用していました。私がコーディングフェイクパックスを扱っていないことを知っていて、ストイックメソッドがどこから来たのですか?ありがとう! – Travis

4

とにかくチェックしなければならないのなら、それは一番上でやるのはいいアイデアだと思います。

(1)不必要な作業を避けるためにチェックを行うか、(2)チェックをスキップして常に作業を行うかを選択できるケースがあります。そのような場合は、作業中でないことが稀であると思う場合は、チェックをスキップすることを検討することもできます。余分なチェックは、時には不要な作業よりもコストがかかります。 )。それでは、どちらが当てはまるかははっきりしないかもしれません。私は単純なif文がかなり安いと言うでしょう(チェック自体は高価ではないと仮定して)、チェックを行い、パフォーマンス上の問題がなければあまり心配しないでください。後でいつでもプロファイリングを行うことができます。

編集:あなたが心の中でさまざまな問題があったように、あなたのさらなる例に基づいて、それが聞こえます。あなたは

if positive_case: 
    lots of stuff 
    lots more stuff 
    ... 
else: 
    whatever this corresponds to is now off the screen 

あなたの技術のようなものを持っていないので、その点について、私は、あなたが一般的に最初の最短のコードをケースに入れなければならないと言うだろう、あまりにも多くのネスティングを回避するための一般的な方法です。 ifから出てくる、とelseを除外することで、あなたは

if error: 
    raise exception 

do more stuff 
if error: 
    ... 

if error: 
    raise exception 
else: 
    do more stuff 
    if error: 
    ... 

を平らにすることができます私はあなたが示唆したように、PEP-8は、実際にこの技術を言及し、推奨していますね。なぜあなたがこれが礼儀正しくないのだろうと思っているのか分かりません。多くのものは個人的な好みに過ぎず、いずれの方法でも賛否両論があります。トレードオフについてのあなた自身の意見があります。

+1

ありがとう!私はトップに向かって最短の条件を持ち、コンディションが体の中でもっと仕事をしているという考えが好きです。 私はそれが一般的な行為であることを知らなかった。私はしばらくの間、長いコードを見てきたと思います:) 私はPEP-8について79文字/行のアスペクトについて言及しましたが、これは推奨された技術と考えられていました。 ありがとうございます! – Travis

関連する問題