#2900 deprecate duplicable? and remove it from the framework - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#2900 ✓wontfix

deprecate duplicable? and remove it from the framework

Reported by Xavier Noria | July 11th, 2009 @ 01:40 AM | in 2.x

Active Support has a predicate called duplicable? that returns true/false according to a hard-coded list of cases. For example strings are duplicable? but numbers are not.

That's used in the framework to conditionally call dup/clone on arbitrary values. However, classes have means to disallow duplication, like removing dup/clone or raising exceptions from them, so that technique is not generic enough.

This patch replaces usage of duplicable? in Rails with a robust rescue idiom, and deprecates the predicate itself.

deprecate_duplicable.diff 6.2 KB

Comments and changes to this ticket

Michael Koziarski August 3rd, 2009 @ 06:13 AM
rescues are quite expensive, this is why we added the duplicable thing in the first place ;)

Not sure what the right fix is here, what values have you seen this being called on?
Xavier Noria November 15th, 2009 @ 08:21 PM
My only concern is that the current code does not play by the rules for arbitrary values.

Deviating from standard Ruby approaches has to be very justified in my view. If experience shows the current approach works in practice, and that avoiding rescue in those points pays off in terms of performance, then no prob (I can't personally tell).

In that case I'd perhaps document the motivation in the rdoc of duplicable?.
Matt Jones November 15th, 2009 @ 09:31 PM
Looks like it's primarily used in ActiveRecord (2.3.4) in clone_attribute_value, which gets called by the dirty mechanism. So removing it in favor of rescue would impose additional execution cost on pretty much every attribute assignment that isn't duplicable (ie, integers and booleans).
Xavier Noria November 15th, 2009 @ 09:35 PM
Yes, and in a few more places (see the patch).
Xavier Noria November 18th, 2009 @ 11:18 PM
I did some isolated benchmarks and added this explanation in duplicable.rb:

http://github.com/lifo/docrails/commit/2ddbef421cb877bc219ac2737bbb...

Please feel free to close the ticket if the comment looks fine.
Xavier Noria February 26th, 2010 @ 05:10 PM
- State changed from “new” to “wontfix”