7

先日、Rubyのコード品質向上ツールを探していたら、面白そうなpelusa gemに出会いました。チェック対象の 1 つは、特定の Ruby ファイルで使用されている else ステートメントの数です。

私の質問は、なぜこれらが悪いのですか？ステートメントが非常に複雑になることが多いことは理解していif/elseます (そして、コードの複雑さを軽減することが目標であることがわかります) が、? なしで 2 つのケースをチェックするメソッドをどのように記述することができelseますか?

要約すると、2 つの質問があります。

1) コードの複雑さを軽減する以外に、else ステートメントを回避できる理由はありますか?

else2) これは、ステートメントを使用する、私が取り組んでいるアプリのサンプル メソッドです。これなしでどうやってこれを書くの？私が考えることができる唯一のオプションは三項ステートメントですが、ここには十分なロジックがあり、三項ステートメントは実際にはより複雑で読みにくいと思います。

def deliver_email_verification_instructions
  if Rails.env.test? || Rails.env.development?
    deliver_email_verification_instructions!
  else
    delay.deliver_email_verification_instructions!
  end
end

これを三項演算子で書くと、次のようになります。

def deliver_email_verification_instructions
  (Rails.env.test? || Rails.env.development?) ? deliver_email_verification_instructions! : delay.deliver_email_verification_instructions!
end

そうですか？もしそうなら、その方が読みにくくありませんか？else声明はこれを打破するのに役立ちませんか？else私が考えていない、これを書くための別の、より良い、より少ない方法はありますか?

私はここでスタイル上の考慮事項を探していると思います。

4

2 に答える 2

6

まず、あなたのコードには実際には何も問題はないということから始めましょう。一般に、コード品質ツールが何を言おうと、まったくナンセンスである可能性があることを認識しておく必要があります。なぜなら、コード品質ツールには、実際に行っていることを評価するためのコンテキストが欠けているからです。

しかし、コードに戻ります。スニペットが

if Rails.env.test? || Rails.env.development?
    # Do stuff
else
    # Do other stuff
end

発生した場合、それはまったく問題ありません (特定のことには常にさまざまなアプローチがありますが、プログラマーがそれについて議論しないことであなたを嫌うとしても、それについて心配する必要はありません:D)。

ここで、トリッキーな部分が来ます。人々は非常に怠け者であり、したがって、上記のようなコード スニペットは、コピー アンド ペースト コーディングの簡単なターゲットになります (これが、最初から避けるべきだと人々が主張する理由です。クラスを後で拡張すると、実際にリファクタリングするのではなく、コピーして貼り付けるだけです）。

例として、コード スニペットを見てみましょう。私は基本的に@Mik_Dieと同じことを提案していますが、彼の例はあなたのものと同じようにコピー/貼り付けされる傾向があります. したがって、行う必要があります（IMO）は次のとおりです。

class Foo
  def initialize
    @target = (Rails.env.test? || Rails.env.development?) ? self : delay
  end

  def deliver_email_verification_instructions
    @target.deliver_email_verification_instructions!
  end
end

このままではアプリに当てはまらないかもしれませんが、「同じことを繰り返さないでください」というアイデアが得られることを願っています。これまで。繰り返すたびに、コードの保守性が低下するだけでなく、その結果、将来的にエラーが発生しやすくなります。残りの1つの発生は@disasterOfEpicProportions、最終的に原因となります:)

私が忘れていたもう 1 つのポイントは、@RayToal (ありがとう :) によって提起されました。これは、if/else コンストラクトがブール入力パラメーターと組み合わせて使用​​されることが多く、このようなコンストラクト (私が持っているプロジェクトの実際のコード) になることです。維持する):

class String
  def uc(only_first=false)
    if only_first
      capitalize
    else
      upcase
    end
  end
end

ここでは、明らかなメソッドの命名とモンキー パッチの問題を無視してuc、パラメーターに応じてメソッドに 2 つの異なる動作を与えるために使用される if/else 構造に注目しましょうonly_first。このようなコードは、メソッドが複数のことを行っているため、単一責任の原則に違反しています。そのため、最初に 2 つのメソッドを記述する必要がありました。

于 2012-12-19T00:17:15.247 に答える
2 
def deliver_email_verification_instructions
  subj = (Rails.env.test? || Rails.env.development?) ? self : delay
  subj.deliver_email_verification_instructions!
end
于 2012-12-18T18:43:20.270 に答える