Permalink
Browse files

Changed ActiveSupport::Memoizable API to extend since it mainly adds …

…the memoize class method
  • Loading branch information...
josh committed Jul 18, 2008
1 parent e1f23da commit ef6f6625c91ea789a033799f649e4388e4a71045
@@ -3,12 +3,12 @@ module Renderable
# NOTE: The template that this mixin is beening include into is frozen
# So you can not set or modify any instance variables
+ extend ActiveSupport::Memoizable
+
def self.included(base)
@@mutex = Mutex.new
end
- include ActiveSupport::Memoizable
-
def filename
'compiled-template'
end
@@ -3,7 +3,7 @@ module RenderablePartial
# NOTE: The template that this mixin is beening include into is frozen
# So you can not set or modify any instance variables
- include ActiveSupport::Memoizable
+ extend ActiveSupport::Memoizable
def variable_name
name.sub(/\A_/, '').to_sym
@@ -1,7 +1,7 @@
module ActionView #:nodoc:
class Template
extend TemplateHandlers
- include ActiveSupport::Memoizable
+ extend ActiveSupport::Memoizable
include Renderable
attr_accessor :filename, :load_path, :base_path, :name, :format, :extension
@@ -1,37 +1,38 @@
module ActiveSupport
- module Memoizable
- def self.included(base) #:nodoc:
- base.extend(ClassMethods)
- end
-
- module ClassMethods
- def memoize(*symbols)
- symbols.each do |symbol|
- original_method = "unmemoized_#{symbol}"
- memoized_ivar = "@#{symbol}"
- raise "Already memoized #{symbol}" if instance_methods.map(&:to_s).include?(original_method)
-
- alias_method original_method, symbol
- class_eval <<-EOS, __FILE__, __LINE__
- def #{symbol}(reload = false)
- if !reload && defined? #{memoized_ivar}
- #{memoized_ivar}
- else
- #{memoized_ivar} = #{original_method}.freeze
- end
+ module Memoizable #:nodoc:
+ def self.extended(obj)
+ klass = obj.respond_to?(:class_eval) ? obj : obj.metaclass
+ klass.class_eval do
+ def freeze
+ methods.each do |method|
+ if m = method.to_s.match(/^unmemoized_(.*)/)
+ send(m[1])
end
- EOS
+ end
+ super
end
end
end
- def freeze
- methods.each do |method|
- if m = method.to_s.match(/\Aunmemoized_(.*)/)
- send(m[1])
- end
+ def memoize(*symbols)
+ symbols.each do |symbol|
+ original_method = "unmemoized_#{symbol}"
+ memoized_ivar = "@#{symbol}"
+
+ klass = respond_to?(:class_eval) ? self : self.metaclass
+ raise "Already memoized #{symbol}" if klass.instance_methods.map(&:to_s).include?(original_method)
+
+ klass.class_eval <<-EOS, __FILE__, __LINE__
+ alias_method :#{original_method}, :#{symbol}
+ def #{symbol}(reload = false)
+ if !reload && defined? #{memoized_ivar}
+ #{memoized_ivar}
+ else
+ #{memoized_ivar} = #{original_method}.freeze
+ end
+ end
+ EOS
end
- super
end
end
end
@@ -3,21 +3,24 @@
uses_mocha 'Memoizable' do
class MemoizableTest < Test::Unit::TestCase
class Person
- include ActiveSupport::Memoizable
+ extend ActiveSupport::Memoizable
def name
fetch_name_from_floppy
end
+ memoize :name
+
def age
nil
end
- def random
- rand(0)
+ def counter
+ @counter ||= 0
+ @counter += 1
end
- memoize :name, :age, :random
+ memoize :age, :counter
private
def fetch_name_from_floppy
@@ -37,9 +40,9 @@ def test_memoization
end
def test_reloadable
- random = @person.random
- assert_equal random, @person.random
- assert_not_equal random, @person.random(:reload)
+ counter = @person.counter
+ assert_equal 1, @person.counter
+ assert_equal 2, @person.counter(:reload)
end
def test_memoized_methods_are_frozen
@@ -58,5 +61,30 @@ def test_memoization_frozen_with_nil_value
def test_double_memoization
assert_raise(RuntimeError) { Person.memoize :name }
end
+
+ class Company
+ def name
+ lookup_name
+ end
+
+ def lookup_name
+ "37signals"
+ end
+ end
+
+ def test_object_memoization
+ company = Company.new
+ company.extend ActiveSupport::Memoizable
+ company.memoize :name
+
+ assert_equal "37signals", company.name
+ # Mocha doesn't play well with frozen objects
+ company.metaclass.instance_eval { define_method(:lookup_name) { b00m } }
+ assert_equal "37signals", company.name
+
+ assert_equal true, company.name.frozen?
+ company.freeze
+ assert_equal true, company.name.frozen?
+ end
end
end

4 comments on commit ef6f662

Contributor

matthewrudy replied Jul 19, 2008

oh wow.
this includes my :comment saying “it should have a force_reload param”

Contributor

matthewrudy replied Jul 19, 2008

oh wow.
this includes my :comment saying “it should have a force_reload param”

Member

josh replied Jul 19, 2008

I really want to accept args too, but it doesn’t mix well with reload. Too bad 1.8 does not support “method(*args, reload = false)”

Member

josh replied Jul 19, 2008

I really want to accept args too, but it doesn’t mix well with reload. Too bad 1.8 does not support “method(*args, reload = false)”

Please sign in to comment.