Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Deprecate memoizable.

  • Loading branch information...
commit 36253916b0b788d6ded56669d37c96ed05c92c5c 1 parent 6c3e80a
@josevalim josevalim authored
View
7 activesupport/lib/active_support/memoizable.rb
@@ -1,8 +1,15 @@
require 'active_support/core_ext/kernel/singleton_class'
require 'active_support/core_ext/module/aliasing'
+require 'active_support/deprecation'
module ActiveSupport
module Memoizable
+ def self.extended(base)
+ ActiveSupport::Deprecation.warn "ActiveSupport::Memoizable is deprecated and will be removed in future releases," \
+ "simply use Ruby instead.", caller
@ryanb
ryanb added a note

By "use Ruby instead" do you mean use @var ||= ...? Memoizable offered some neat features such as using a different cache for different arguments, handling nil better, etc.

@dedene
dedene added a note

Hi Ryan,

Did you get any answer to this question?

I'm also curious to what is meant with "use Ruby instead".

Many thanks in advance,
Peter

@thoefer
thoefer added a note

You should use Ruby´s @var ||= pattern instead... see comments below for more information

Sometimes you want 'nil' to be a valid 'defined' value, which Memoizable handles but ||= doesn't, in which case I recommend:

     @foo = whatever unless defined? @foo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ super
+ end
+
def self.memoized_ivar_for(symbol)
"@_memoized_#{symbol.to_s.sub(/\?\Z/, '_query').sub(/!\Z/, '_bang')}".to_sym
end
View
4 activesupport/test/flush_cache_on_private_memoization_test.rb
@@ -2,7 +2,9 @@
require 'test/unit'
class FlashCacheOnPrivateMemoizationTest < Test::Unit::TestCase

should be FlushCacheOnPrivateMemoizationTest I believe

@gyebaek
gyebaek added a note

작성 합니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- extend ActiveSupport::Memoizable
+ ActiveSupport::Deprecation.silence do
+ extend ActiveSupport::Memoizable
+ end
def test_public
assert_method_unmemoizable :pub
View
24 activesupport/test/memoizable_test.rb
@@ -2,7 +2,9 @@
class MemoizableTest < ActiveSupport::TestCase
class Person
- extend ActiveSupport::Memoizable
+ ActiveSupport::Deprecation.silence do
+ extend ActiveSupport::Memoizable
+ end
attr_reader :name_calls, :age_calls, :is_developer_calls, :name_query_calls
@@ -65,7 +67,9 @@ def name
end
module Rates
- extend ActiveSupport::Memoizable
+ ActiveSupport::Deprecation.silence do
+ extend ActiveSupport::Memoizable
+ end
attr_reader :sales_tax_calls
def sales_tax(price)
@@ -77,7 +81,9 @@ def sales_tax(price)
end
class Calculator
- extend ActiveSupport::Memoizable
+ ActiveSupport::Deprecation.silence do
+ extend ActiveSupport::Memoizable
+ end
include Rates
attr_reader :fib_calls
@@ -215,7 +221,9 @@ def test_memoization_with_boolean_arg
def test_object_memoization
[Company.new, Company.new, Company.new].each do |company|
- company.extend ActiveSupport::Memoizable
+ ActiveSupport::Deprecation.silence do
+ company.extend ActiveSupport::Memoizable
+ end
company.memoize :name
assert_equal "37signals", company.name
@@ -249,11 +257,15 @@ def test_object_memoized_module_methods
def test_double_memoization
assert_raise(RuntimeError) { Person.memoize :name }
person = Person.new
- person.extend ActiveSupport::Memoizable
+ ActiveSupport::Deprecation.silence do
+ person.extend ActiveSupport::Memoizable
+ end
assert_raise(RuntimeError) { person.memoize :name }
company = Company.new
- company.extend ActiveSupport::Memoizable
+ ActiveSupport::Deprecation.silence do
+ company.extend ActiveSupport::Memoizable
+ end
company.memoize :name
assert_raise(RuntimeError) { company.memoize :name }
end

38 comments on commit 3625391

@josh
Collaborator

:zap:

@karlfreeman

Thought I'd let you know this change breaks Haml and Devise, will file bug reports on both as they both have dependencies that conflict with this change.

DEPRECATION WARNING: ActiveSupport::Memoizable is deprecated and will be removed in future releases,simply use Ruby instead. (called from module:Routing at /Users/karlfreeman/.rvm/gems/ruby-1.9.2-p180@**/gems/devise-1.3.4/lib/devise/rails/routes.rb:12)

DEPRECATION WARNING: ActiveSupport::Memoizable is deprecated and will be removed in future releases,simply use Ruby instead. (called from module:ActionView at /Users/karlfreeman/.rvm/gems/ruby-1.9.2-p180@**/gems/haml-3.1.2/lib/haml/helpers/action_view_mods.rb:2)

@sikachu
Collaborator

I wish it would say "simply use Ruby's way of memoization instead." More clear.

@sikachu
Collaborator

And no CHANGELOG? Bro, please make our life (especially @tenderlove) easier by adding CHANGELOG for some change like this.

@samgranieri

why is this getting pulled?

@bquorning

I too am wondering why this is being removed from Rails? Like @ryanb said above, it offers some nice features such as argument aware caching.

@dmathieu
Collaborator

It does indeed needs much more documentation :(

@thoefer

I would also be interested in the driving thoughts behind this change

@josevalim
Owner

And no CHANGELOG? Bro, please make our life (especially @tenderlove) easier by adding CHANGELOG for some change like this.

Calm down bro, I forgot, just remind me. I am aware how important the CHANGELOG changes are. :)

@josevalim
Owner

Updated CHANGELOG and improved the message in 8775ffa as @sikachu suggest. Tks for watching my back bro <3

@josevalim
Owner

This is being removed because during the Rails 3 refactoring we have removed almost all occurrences of Memoizable for the simpler ruby @var ||= pattern. As this is no longer used and encouraged in Rails, I decided to deprecate it. If someone really needs this, someone can gemify it.

@thoefer

Ok, it makes sense to me too as it removes a couple of meta techniques used under the hood and makes things more explicit. Thanks for you explanation, José!

@josevalim
Owner

Examples of memoize being replaced by simpler and better code: f2c0fb3

@thoefer

It seems to me that all occurences are removed except Memoization itself and some tests for it. If you want to try yourself -> find . -name "." | xargs grep "Memoizable"

@pda

This commit has just hit the release notes for Rails 3.2.

I can't see how ActiveSupport::Memoizable can be deprecated in favour of @var ||= ...; they're not equivalent.

ActiveSupport::Memoizable nicely packages up some logic which is tedious and error-prone to repeatedly implement. It correctly memoizes non-true values (nil, false etc) and varies memoization by method parameters. As a bonus, it keeps cached return values separate from actual instance state; two semantically distinct types of data.

Removing Memoizable would be like removing ActiveSupport::Concern - the same result can be achieved by "simply using Ruby", but it's a valuable part of ActiveSupport.

Cheers,
Paul

@josevalim
Owner
@sj26

I do not like this deprecation.

The reason Memoize is complex is to satisfy all the cases we end up building into manual memoization code. They are a wonderful encapsulation of that knowledge which even a novice may use.

... ||= ... is not Hash.new { ... } is not @... = ... unless defined? ..., the differences of which may elude less advanced ruby developers, yet all can be simply and elegantly expressed by the memoize decorator.

One piece of knowledge in ActiveSupport::Memoize is that straight hash memoization is cheaper than hash with a default block.

Look at how simply it started and the knowledge we will lose by removing it.

@josevalim
Owner
@josevalim
Owner
@jrochkind
@Bodacious

I don't mind this being deprecated.

I for most cases the @var ||= or return @var if defined?(@var) ... approaches are sufficient. Also, a lot of less experienced developers miss out on the how and why of these important concepts when it's all done under the hood by methods like memoize.

Can someone please change the deprecation message though? There is no single "Ruby memoization pattern" so this message is misleading.

`DEPRECATION WARNING: ActiveSupport::Memoizable is deprecated and will be removed in future releases,simply use Ruby memoization pattern instead. `
@Aaron2Ti

The @var ||= way could not cope with functions which having arguments, so I wrote a small gem memor to solve these problems:

class Foo
  def bar(arg1, arg2, *args)  # have to cache this method
     do_something     
  end
end

In order to cache Foo#bar just wrap the existed function inside a memor block:

require 'memor'

class Foo
  include Memor

  def bar(arg1, arg2, *args) # cached per arguments
    memor binding do # memor wrapper 
      do_something
    end     
  end
end

We used this gem in one client app to cache heavy statics calculations for reports.

@joaomilho

As a hard core Haskell fan I think Memoize should not be deprecd but encouraged. ||= has NOTHING to do w/ Memoize (as everyone knows). I can understand that it is what we need most of the time. But people would learn a lot if Memoize is still core (at least my students do).

@alk

memoize is very useful to cache some automatically built methods. Like the ones produced by #delegate

Sad to see it's gone.

@jrochkind
@s01ipsist

Someone did take it and make it into a gem
https://github.com/matthewrudy/memoist

@gamov

Sometimes Rails core decisions are not understandable for the mere mortals that we are.... this is one.

@roryokane

I think this was a stupid decision. I’m glad Memoist exists so we can still use memoize in our own code.

Patterns are made to be encapsulated. Memoizable turns this pattern:

def something
  @cached_something ||= big_calculation
end

into this:

def something
  big_calculation
end
memoize :something

You could call the second use a pattern too – but the idea is that the use of the encapsulation is simpler than its re-implementation. I would say that that is definitely the case here.

The first example uses ||= in a way that doesn’t match its name. When you look at the symbol ||=, the first thing you think of is its name “or-equals”. Of course, experienced Ruby developers will remember right after that that ||= almost always means memoization. But why go the long way around to the concept of memoization when you can actually have real, literal memoization? “memoize” – it does what is says, and is easily and quickly understandable. There is the danger that people won’t know what memoize means – but they’re not going to know what ||= means, either. And I haven’t even mentioned the extra features memoize also provides yet.

josevalim justified the change with this:

we have removed almost all occurrences of Memoizable for the simpler Ruby @var ||= pattern

But memoize is simpler than ||=. It cleanly means “memoize”, where ||= means “memoize (under a different name), kind of, except in certain situations where it will fail”. The only way you might try to say ||= is simpler is that it is built into the language, so people be more likely to recognize it. But nobody in Ruby will recognize the function of ||= on their own, without being told about it first. And with all the effort of developer education you do to teach people that ||= means memoization, you could just tell them to use memoize.

@roryokane

A Stack Overflow question about this, “Which Ruby memoize pattern does ActiveSupport::Memoizable refer to?”, links here. Its answers include links to more information, ways to use memoization now, and someone’s interpretation of the reasons for this change.

@joaomilho

"experienced Ruby developers will remember right after that that ||= almost always means memoization" ... "But memoize is simpler than ||=" . I don't think so. When I think about memoize in Ruby I think "||= means memoization when there are no params" which is a lil bit more complicated. You just can't do that:

def sum a,b
  @result ||= a + b
end
sum 2,2 => 4
sum 2,3 => 4

You gotta do something like this:

def sum a, b
  @results ||= {}
  @results[[a,b]] ||= a+b
end
@roryokane

@joaomilho I think we are in agreement. I alluded to that problem later in my comment:

And I haven’t even mentioned the extra features memoize also provides yet.

and

||= means “memoize (under a different name), kind of, except in certain situations where it will fail”

What you describe is indeed another flaw of ||= that memoize fixes. I was focusing my comment on just the problem of ||=’s name, because I thought that that flaw hadn’t been explained enough in these comments yet. I do also like that memoize can memoize methods with parameters.

(By the way, in GitHub comments, write code inline like `this` and code blocks like this:

```ruby
# Ruby code here
```
@joaomilho

Ok @roryokane , thanks for the github comments tips. It's just I'm too used to Haskell to hear that we should throw away memoize somewhere else and "just use ||=" as if the former is the same as the later! :D

@juniorz

@joaomilho, In either case you just used plain ruby. That is what the deprecation message says.

@denispeplin

Two days ago spent some time implementing binary caching for CanCan. Just discovered from CodeSchool 'Best Practices' that it was a better way: memoize. But it was gone. Sad.

@jrochkind

If you want what used to be ActiveSupport::Memoizable, just take the file from before it was deleted, put it in your project, and use it. It's right here: https://github.com/rails/rails/blob/36253916b0b788d6ded56669d37c96ed05c92c5c/activesupport/lib/active_support/memoizable.rb

It's only 117 lines!

Or even do that and put it in a gem. Has nobody done that yet?

All you lose by it not being in Rails anymore, is the Rails team's commitment to maintain it for you. Sorry, they didn't need it anymore, decided for them it wasn't the right solution, had to prioritize, and decided they would no longer be maintaining it.

@denispeplin

I will use the memoist gem: https://github.com/matthewrudy/memoist

IMHO, the sad part about moving memoization outside Rails, is that this method will be not mentioned in Rails documentation. Many Rails developers will invent bicycles again over and over.

@jrochkind

But even though you were using Rails all along, you didn't discover it from the Rails documentation, you discovered it from Code School, and had been working on re-inventing it before you noticed it there.

In fact, it was never really mentioned in the rails documentation, the module didn't even have any comments for rdoc. I agree that if it had remained in rails, it should have been documented. Instead, it's documented in memoist, great.

If you want to avoid re-inventing what a gem can do for you, you shouldn't restrict yourself to only what's in Rails. (And certainly sometimes it makes sense to write something short yourself to your own requirements instead of always trying to use an abstracted general purpose gem; although even those cases looking at source code for the gem you don't use can be helpful, certainly)

@bencao

For guys doing cache on ActiveRecord models, I got another implementation acts_as_method_cacheable which offers the ability to:

  • cache method for instances
post = Post.find(xxx)
post.cache_method(:expensive_method)
post.cache_method([:expensive_method1, :expensive_method2])
  • cache methods on associations, using eager load 'includes' like syntax
post = Post.find(xxx)
post.cache_method([:expensive_method3, :comments => :comment_expensive_method])
  • automatically clear cache after reload

Inside the implementation is a unless @defined @_iVal, so methods return nil and false are well supported.

A important motivation of doing this is to do cache on CONTROLLER level other than do it in MODELs. Silently doing cache on model make it buggy for callers, such as this case:

class Post
  def expensive
    @cached_expensive ||= _expensive
  end

  def _expensive
    @title + @author_name
  end
end

post = Post.find(xxx)
post.expensive

post.title = 'new title'
post.save!

post.expensive      # return old title

The ideal case is callers do not have to know details of Post to make their programs bug free.

Please sign in to comment.
Something went wrong with that request. Please try again.