1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Shorter code: remove unnecessary condition

See 136fc65c9b (r28897107)

 I _think_ that this method can now be rewritten from:

```ruby
def attribute_previous_change(attr)
  previous_changes[attr] if attribute_previously_changed?(attr)
end
```

to:

```ruby
def attribute_previous_change(attr)
  previous_changes[attr]
end
```

without losing performance.

---

Calling

```ruby
previous_changes[attr] if attribute_previously_changed?(attr)
```

is equivalent to calling

```ruby
previous_changes[attr] if previous_changes.include?(attr)
```

When this commit 136fc65c9b was made, Active Record had its own `previous_changes` method, added here below. However, that method has been recently removed from the codebase, so `previous_changes` is now only the method defined in Active Model as:

```ruby
def previous_changes
  @previously_changed ||= ActiveSupport::HashWithIndifferentAccess.new
  @previously_changed.merge(mutations_before_last_save.changes)
end
```

Since we are dealing with a memoized Hash, there is probably no need to check `if .include?(attr_name)` before trying to fetch `[attr]` for it.

Does that make sense? Did I miss anything? Thanks!
This commit is contained in:
claudiob 2018-07-05 06:08:31 -07:00
parent ba38e13c82
commit 0cd36e2b22

View file

@ -304,7 +304,7 @@ module ActiveModel
# Handles <tt>*_previous_change</tt> for +method_missing+.
def attribute_previous_change(attr)
previous_changes[attr] if attribute_previously_changed?(attr)
previous_changes[attr]
end
# Handles <tt>*_will_change!</tt> for +method_missing+.