1

モデルの保存時にオプションであるいくつかの属性を持つモデルがあります。

これらの属性を使用して計算を実行するいくつかのインスタンス メソッドがありますが、最初にそれらが nil でないかどうかを確認したいと思います。no method error nil nil class

私のコードをポイ捨てする以外に、.present?これを行うためのより良い方法はありますか?

編集: これまでの私のコードは次のとおりです

def is_valid?
   (has_expired? === false and has_uses_remaining?) ? true : false
end

def has_expired?
   expiry_date.present? ? expiry_date.past? : false
end

def remaining_uses
  if number_of_uses.present?
    number_of_uses - uses_count
  end
end

def has_uses_remaining?
  number_of_uses.present? ? (remaining_uses > 0) : true
end

チェックを実行するために立ち寄ると.present?コードの臭いがするように感じます。null オブジェクト パターンを調べましたが、オブジェクトが存在するため、ここでは意味がないように見えますが、属性の一部はnil

4

2 に答える 2

1

通常、これらの状況では短絡が最も効果的です。

前：

if @attribute.present?
  @attribute.do_something
end

短絡:

@attribute && @attribute.do_something

&&ショートサーキットのアプローチでは、Ruby は演算子の左側がnilであることを確認するとすぐに停止し、右側を実行しません。

また、特定の属性が許可されるべき理由についてもよく考えますnil(Jordan が尋ねたように)。これを回避する方法を考えることができれば、それはより良いかもしれません。

になりたいと仮定すると、次のように書き換えることができnumber_of_usersます。nilhas_uses_remaining?

def has_uses_remaining?
  !number_of_uses || remaining_uses > 0
end

-補足: 最初の方法は次のように簡略化できます。

def is_valid?
   !has_expired? && has_uses_remaining?
end
于 2016-01-03T02:12:03.897 に答える
1

ここでの本当の問題は、（あなたが発見したように）非常に複雑にnumber_of_usesなる可能性があることだと思います。nilまずその問題を解消するようにしてください。

何らかの理由でそれができない場合は、各方法を改善できます。

  1. condition ? true : false常にコードのにおいがします。Boolean 演算子は boolean(ish) 値を返すので、その仕事をさせてください:

    def is_valid?
  !has_expired? && has_uses_remaining?
end

  2. 個人的には、Rails を使用するのは通常、コードの匂いがすると思いますObject#tryが、ここでは非常に適しています。

    def has_expired?
  expiry_date.try(:past?)
end

    または:

    def has_expired?
  expiry_date.present? && expiry_date.past?
end

  3. これを大幅に改善することはできませんが、個人的には、ブロックreturnにラップされたメソッドよりも早い方が好きです。if

    def remaining_uses
  return if number_of_uses.nil?
  number_of_uses - uses_count
end

    あなたもすることができますnumber_of_uses && number_of_uses - uses_count（または、number_of_uses.try(:-, uses_count)しかし私はこれがより明確だと思います.

  4. このメソッドがビットのtrue場合に返すのは少し奇妙です。これは、次のように単純化できるためです。number_of_usesnil

    def has_uses_remaining?
  remaining_uses.nil? || remaining_uses > 0
end

    ;remaining_uses.nil?の代わりに呼び出すことに注意してください。number_of_uses.nil?一方から同じ結果が得られる場合、両方に依存する必要はありません。

さらなる改善

さらに検討すると、別の方法を導入することで、このコードの意図をより明確にすることができると思います: has_unlimited_uses?:

def has_unlimited_uses?
  number_of_uses.nil?
end

def is_valid?
  !has_expired? &&
    has_unlimited_uses? || has_uses_remaining?
end

def remaining_uses
  return if has_unlimited_uses?
  number_of_uses - uses_count
end

def has_uses_remaining?
  has_unlimited_uses? || remaining_uses > 0
end

このようにして、チェック対象についてあいまいさがなくなります。これにより、コードを次に読む人 (または今から 6 か月後のあなた) にとってコードが読みやすくなり、バグの追跡が容易になります。

しかし、それはまだ私を悩ませていremaining_usesますnil。代わりに を返すFloat::INFINITYと、has_uses_remaining?単純な比較になることがわかります。

def remaining_uses
  return Float::INFINITY if has_unlimited_uses?
  number_of_uses - uses_count
end

def has_uses_remaining?
  remaining_uses > 0
end
于 2016-01-03T02:54:26.790 に答える