1

我有一个模型，它有几个在保存模型时是可选的属性。

我有几个使用这些属性并执行计算的实例方法，但我想先检查它们是否不为零，因为我会得到可怕的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?插入执行检查有一种不好的代码气味，我已经研究了空对象模式，但在这里似乎没有意义，因为对象存在，但一些属性是nil

4

2 回答 2

1

在这些情况下，短路通常效果最好。

前：

if @attribute.present?
  @attribute.do_something
end

短路：

@attribute && @attribute.do_something

使用短路的方法，Ruby一看到&&运算符的左侧是nil，它就会停止而不运行右侧

我也会认真思考为什么应该允许某个特定属性存在nil（正如乔丹所问的那样）。如果你能想办法避免这种情况，那可能会更好。

假设您确实希望number_of_users能够成为nil，您可以这样重写has_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_usescan be nil，它（正如您所发现的）引入了大量的复杂性。首先尝试消除该问题。

如果由于某种原因您不能这样做，则可以改进您的每种方法：

  1. condition ? true : false始终是代码气味。布尔运算符返回 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. 这个方法返回trueif number_of_usesis nilbit 有点奇怪，因为我们可以像这样简化它：

    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

这样，您要检查的内容就不会有任何歧义。这将使代码对于下一个阅读它的人（或六个月后的你）更具可读性，并使跟踪错误更容易。

但是，它仍然困扰着我，remaining_uses返回nil. 事实证明，如果我们改为 return 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 回答