Skip to content

Commit

Permalink
much code can be deleted thanks to @tenderlove's refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
jonleighton committed Mar 28, 2012
1 parent 9897b9a commit 6cff090
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 72 deletions.
8 changes: 0 additions & 8 deletions activerecord/lib/active_record/attribute_methods.rb
Expand Up @@ -49,14 +49,6 @@ def attribute_methods_generated?
@attribute_methods_generated ||= false
end

# We will define the methods as instance methods, but will call them as singleton
# methods. This allows us to use method_defined? to check if the method exists,
# which is fast and won't give any false positives from the ancestors (because
# there are no ancestors).
def generated_external_attribute_methods
@generated_external_attribute_methods ||= Module.new { extend self }
end

def undefine_attribute_methods
super if attribute_methods_generated?
@attribute_methods_generated = false
Expand Down
Expand Up @@ -43,12 +43,6 @@ def define_method_attribute(attr_name)

if attr_name == primary_key && attr_name != 'id'
generated_attribute_methods.send(:alias_method, :id, primary_key)
generated_external_attribute_methods.module_eval <<-CODE, __FILE__, __LINE__
def id(v, attributes, attributes_cache, attr_name)
attr_name = '#{primary_key}'
send(attr_name, attributes[attr_name], attributes, attributes_cache, attr_name)
end
CODE
end
end

Expand Down
64 changes: 6 additions & 58 deletions activerecord/lib/active_record/attribute_methods/read.rb
Expand Up @@ -29,35 +29,8 @@ def cache_attribute?(attr_name)
cached_attributes.include?(attr_name)
end

def undefine_attribute_methods
generated_external_attribute_methods.module_eval do
instance_methods.each { |m| undef_method(m) }
end

super
end

def type_cast_attribute(attr_name, attributes, cache = {}) #:nodoc:
return unless attr_name
attr_name = attr_name.to_s

if generated_external_attribute_methods.method_defined?(attr_name)
if attributes.has_key?(attr_name) || attr_name == 'id'
generated_external_attribute_methods.send(attr_name, attributes[attr_name], attributes, cache, attr_name)
end
elsif !attribute_methods_generated?
# If we haven't generated the caster methods yet, do that and
# then try again
define_attribute_methods
type_cast_attribute(attr_name, attributes, cache)
else
# If we get here, the attribute has no associated DB column, so
# just return it verbatim.
attributes[attr_name]
end
end

protected

# We want to generate the methods via module_eval rather than define_method,
# because define_method is slower on dispatch and uses more memory (because it
# creates a closure).
Expand All @@ -67,51 +40,24 @@ def type_cast_attribute(attr_name, attributes, cache = {}) #:nodoc:
# we first define with the __temp__ identifier, and then use alias method to
# rename it to what we want.
def define_method_attribute(attr_name)
cast_code = attribute_cast_code(attr_name)

generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
def __temp__
#{internal_attribute_access_code(attr_name, cast_code)}
end
alias_method '#{attr_name}', :__temp__
undef_method :__temp__
STR

generated_external_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
def __temp__(v, attributes, attributes_cache, attr_name)
#{external_attribute_access_code(attr_name, cast_code)}
read_attribute('#{attr_name}') { |n| missing_attribute(n, caller) }
end
alias_method '#{attr_name}', :__temp__
undef_method :__temp__
STR
end

private

def cacheable_column?(column)
if attribute_types_cached_by_default == ATTRIBUTE_TYPES_CACHED_BY_DEFAULT
! serialized_attributes.include? column.name
else
attribute_types_cached_by_default.include?(column.type)
end
end

def internal_attribute_access_code(attr_name, cast_code)
"read_attribute('#{attr_name}') { |n| missing_attribute(n, caller) }"
end

def external_attribute_access_code(attr_name, cast_code)
access_code = "v && #{cast_code}"

if cache_attribute?(attr_name)
access_code = "attributes_cache[attr_name] ||= (#{access_code})"
end

access_code
end

def attribute_cast_code(attr_name)
columns_hash[attr_name].type_cast_code('v')
end

This comment has been minimized.

Copy link
@tenderlove

tenderlove Mar 28, 2012

Member

I wonder if we can deprecate or remove type_cast_code

This comment has been minimized.

Copy link
@jonleighton

jonleighton Mar 28, 2012

Author Member

I think so. Will you investigate or shall I?

This comment has been minimized.

Copy link
@tenderlove

tenderlove Mar 28, 2012

Member

I'm pretty busy today, PDI.

This comment has been minimized.

Copy link
@tenderlove

tenderlove Mar 28, 2012

Member

Or add a ticket and I can investigate later. :-)

This comment has been minimized.

Copy link
@carlosantoniodasilva

carlosantoniodasilva Mar 30, 2012

Member

Hey guys, just a follow up, I've added a deprecation warning to type_cast_code, as it's not being used anywhere else in the code base =)

end

# Returns the value of the attribute identified by <tt>attr_name</tt> after it has been typecast (for example,
Expand All @@ -120,7 +66,9 @@ def read_attribute(attr_name)
# If it's cached, just return it
@attributes_cache.fetch(attr_name.to_s) { |name|
column = @columns_hash.fetch(name) {
return self.class.type_cast_attribute(name, @attributes, @attributes_cache)
return @attributes.fetch(name) {
@attributes[self.class.primary_key] if name == 'id'
}

This comment has been minimized.

Copy link
@tenderlove

tenderlove Mar 28, 2012

Member

So these are the magic lines I couldn't figure out! ❤️

}

value = @attributes.fetch(name) {
Expand Down

7 comments on commit 6cff090

@jeremy
Copy link
Member

@jeremy jeremy commented on 6cff090 Mar 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️❤️❤️❤️❤️❤️

@tenderlove
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!

@plentz
Copy link
Contributor

@plentz plentz commented on 6cff090 Mar 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the middle of all this bikeshedding, you guys still rock ❤️

@mtmcfarl
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleting code is so awesome <3!

@nickbuff
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3<3<3!

@lucasprim
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aint no way not to love it! <3 <3 <3!

@ernie
Copy link
Contributor

@ernie ernie commented on 6cff090 Mar 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So much love in this thread. I just want to bask in the positivity. ❤️

Please sign in to comment.