From 0cd36e2b22d9a6ef96364f97046d5fa47afcbc42 Mon Sep 17 00:00:00 2001 From: claudiob Date: Thu, 5 Jul 2018 06:08:31 -0700 Subject: Shorter code: remove unnecessary condition See https://github.com/rails/rails/commit/136fc65c9b8b66e1fb56f3a17f0d1fddff9b4bd0#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! --- activemodel/lib/active_model/dirty.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activemodel') diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index eaf8dfb223..093052a70c 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -304,7 +304,7 @@ module ActiveModel # Handles *_previous_change for +method_missing+. def attribute_previous_change(attr) - previous_changes[attr] if attribute_previously_changed?(attr) + previous_changes[attr] end # Handles *_will_change! for +method_missing+. -- cgit v1.2.3