Skip to content

Commit

Permalink
Update CHANGELOG, improve message.
Browse files Browse the repository at this point in the history
  • Loading branch information
@josevalim
josevalim committed Jun 16, 2011
1 parent 43ed24e commit 8775ffa
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
4 changes: 3 additions & 1 deletion activesupport/CHANGELOG
View file
@@ -1,10 +1,12 @@
*Rails 3.2.0 (unreleased)* *Rails 3.2.0 (unreleased)*


* Deprecated ActiveSupport::Memoizable in favor of Ruby memoization pattern [José Valim]

* Added Time#all_day/week/quarter/year as a way of generating ranges (example: Event.where(created_at: Time.now.all_week)) [DHH] * Added Time#all_day/week/quarter/year as a way of generating ranges (example: Event.where(created_at: Time.now.all_week)) [DHH]


* Added instance_accessor: false as an option to Class#cattr_accessor and friends [DHH] * Added instance_accessor: false as an option to Class#cattr_accessor and friends [DHH]


* Removed ActiveSupport::SecureRandom in favour of SecureRandom from the standard library [Jon Leighton] * Removed ActiveSupport::SecureRandom in favor of SecureRandom from the standard library [Jon Leighton]

This comment has been minimized.

Sign in to view

Copy link
@drogus

drogus Dec 20, 2011

Member

Y U NO LIKE BRITISH ENGLISH?! :(

This comment has been minimized.

Sign in to view

Copy link
@stevegraham

stevegraham Dec 20, 2011

Contributor

AKA actual English.

This comment has been minimized.

Sign in to view

Copy link
@drogus

drogus Dec 20, 2011

Member

:D

This comment has been minimized.

Sign in to view

Copy link
@drogus

drogus Dec 20, 2011

Member

I smell a war ;) /cc @jonleighton

This comment has been minimized.

Sign in to view

Copy link
@josevalim

josevalim Dec 20, 2011

Author Contributor

Aye, I didn't mean to be a pain in the arse. I swear I thought it was a typo. Cheerio.

This comment has been minimized.

Sign in to view

Copy link
@drogus

drogus Dec 20, 2011

Member

I thought so, but not picking it up would not be funny :P

This comment has been minimized.

Sign in to view

Copy link
@josevalim

josevalim Dec 20, 2011

Author Contributor

Bugger off!

This comment has been minimized.

Sign in to view

Copy link
@jonleighton

jonleighton Dec 20, 2011

Member

http://www.youtube.com/watch?v=H3r9bOkYW9s


* ActiveSupport::OrderedHash now has different behavior for #each and * ActiveSupport::OrderedHash now has different behavior for #each and
#each_pair when given a block accepting its parameters with a splat. [Andrew Radev] #each_pair when given a block accepting its parameters with a splat. [Andrew Radev]
Expand Down
2 changes: 1 addition & 1 deletion activesupport/lib/active_support/memoizable.rb
View file
Expand Up @@ -6,7 +6,7 @@ module ActiveSupport
module Memoizable module Memoizable
def self.extended(base) def self.extended(base)
ActiveSupport::Deprecation.warn "ActiveSupport::Memoizable is deprecated and will be removed in future releases," \ ActiveSupport::Deprecation.warn "ActiveSupport::Memoizable is deprecated and will be removed in future releases," \
"simply use Ruby instead.", caller "simply use Ruby memoization pattern instead.", caller
super super
end end


Expand Down

15 comments on commit 8775ffa

@tansengming
Copy link

Choose a reason for hiding this comment

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

Just curious, why is this getting deprecated? I just found out about this module today and thought it was a lot cleaner than littering the model with '||='s

@pboling
Copy link
Contributor

Choose a reason for hiding this comment

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

"simply use Ruby memoization pattern instead."

Memoization is now part of standard Ruby.

@matthewrudy
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm late to the show.
But curious.

@pboling, what does the ruby memoization look like?
Don't think I've seen anything about it.

@pboling
Copy link
Contributor

@pboling pboling commented on 8775ffa Dec 2, 2011

Choose a reason for hiding this comment

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

I should have said that the pattern is now standard, or at least that they consider it standard enough to not need this module.

I have not seen anything defined as the "standard" (by Matz, for example), but in my estimation this would be it:
http://stackoverflow.com/a/963895

def current_user_session
  defined?(@current_user_session) ?
      @current_user_session : @current_user_session = UserSession.find
end

@matthewrudy
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's pretty dirty compared to.

def current_user_session
  UserSession.find
end
memoize :current_user_session

and that's a simple example.

def to_params
  to_hash.map do |key, value|
    "#{key}=#{value}"
  end.join("&")
end
memoize :to_params

looks ok.

But....

def to_params
  if defined?(@to_params)
    @to_params
  else
    @to_params = to_hash.map do |key, value|
      "#{key}=#{value}"
    end.join("&")
  end
end

looks gross.

But I guess what we're really saying is that it doesn't belong in Rails.

Think I'll just put it into a memoizable gem.

@josevalim
Copy link
Contributor Author

Choose a reason for hiding this comment

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

def to_params
  @to_params ||= to_hash.map do |key, value|
    "#{key}=#{value}"
  end.join("&")
end

FTFY.

@matthewrudy
Copy link
Contributor

Choose a reason for hiding this comment

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

I need to think of a more ugly example. :)
Thanks Jose.

@pboling
Copy link
Contributor

@pboling pboling commented on 8775ffa Dec 7, 2011

Choose a reason for hiding this comment

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

My original comment was somewhat sarcastic, and that was lost in transmission.

As someone who uses memoize extensively I am dismayed by this change with not so much as a hint as to how to replicate the behavior. It is emphatically NOT the same as ||= and this is a key reason why we switched to memoize. My example does replicate the behavior for our purposes, but swapping that out for our uses of memoize will be nasty. I will install your gem, so please update this thread if you actually do create it!

Also Rails, why didn't you pull this out into a separate plugin/gem like you have with everything else? Not cool.

@josevalim
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also Rails, why didn't you pull this out into a separate plugin/gem like you have with everything else?

  1. Who did most of the extractions previously were Rails developers, not someone from the Rails Core Team;

  2. Most plugins extracted from Rails are not even in github.com/rails anymore, other people forked and are now maintained them. So we recommend you to do the same if you care about memoize this much;

@dmeremyanin
Copy link
Contributor

Choose a reason for hiding this comment

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

You can also use begin end for more complex cases:

def memoizable
  @memoizable ||= begin
    ...
  end
end

@pboling
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @matthewrudy I just noticed your extraction, and will use it!

@MGPalmer
Copy link

Choose a reason for hiding this comment

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

@pboling, That repo is dead, the nearest thing I could find is https://github.com/JackDanger/simple_memoize .

@matthewrudy
Copy link
Contributor

Choose a reason for hiding this comment

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

@MGPalmer I moved it to https://github.com/matthewrudy/memoist.
This codebase is exactly the same as that in ActiveSupport, except for the namespace.

The tests and authors have been copied straight from the rails project.

@MGPalmer
Copy link

Choose a reason for hiding this comment

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

@matthewrudy thanks !

@roryokane
Copy link
Contributor

Choose a reason for hiding this comment

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

The comments on the commit when ActiveSupport::Memoizable was deprecated also discuss this change.

Please sign in to comment.