This project is archived and is in readonly mode.
composed_of constructor and converter options
Reported by Rob Anderton | August 24th, 2008 @ 11:08 AM
Attached is a patch for composed_of aggregations that does the following:
- It adds a new :constructor option that takes a symbol or a proc that will be used to instantiate the aggregate object. It is optional and if not used then the existing behaviour of calling new with the mapped attributes will be used.
- It deprecates the use of a block to provide a method of converting values assigned to the aggregate attribute. The use of a block didn’t feel particularly consistent with the rest of Rails where typically symbols or procs are passed as options (a good example being the :if and :unless options for validations). Of course passing a block will still function, so existing code won’t break, but it will raise a deprecation warning. This change leads on to…
- It adds a new :converter option that also takes a symbol or proc that will be used when the aggregate attribute is assigned a value that is not an instance of the aggregate class. This replaces the old block syntax and makes it easier to do things like calling composed_of from within a with_options block. If both the :converter option and a block are passed to composed_of then the :converter option will take precedence.
This is a different approach to that taken by previous patches which is why I’m submitting it as a new ticket.
Here’s a simple usage example, currently this won't work:
class Visitor
composed_of :ip_addr,
:class_name => 'IPAddr',
:mapping => %w(ip to_i)
end
Here the composed_of aggregation maps the ip attribute (an integer) to the IPAddr class from the Ruby standard library. This won’t work properly as the IPAddr constructor requires a second parameter to be passed to the constructor. I appreciate that a workaround is to manually instantiate aggregate classes when the model is initialised, but I don’t see why we should have to - it doesn't seem very elegant.
This patch allows the following code to be used:
class Visitor
composed_of :ip_addr,
:class_name => 'IPAddr',
:mapping => %w(ip to_i),
:constructor => Proc.new { |value| IPAddr.new(value, Socket::AF_INET) },
:converter => Proc.new { |value| value.is_a?(Integer) ? IPAddr.new(value, Socket::AF_INET) : IPAddr.new(value.to_s) }
end
Comments and changes to this ticket
-
Michael Koziarski August 26th, 2008 @ 05:01 PM
I don't quite follow what :converter is compared to :constructor
-
Rob Anderton August 26th, 2008 @ 07:20 PM
My IPAddress example is probably to blame. The differences are:
- :constructor is called when instantiating the aggregate object and is therefore passed all of the mapped attributes in the order they are defined in the :mapping option
- :converter is called when a value is assigned to the aggregate attribute and therefore passed the single value that is used in the assignment
Here's an example that hopefully makes this difference more obvious - it uses the NetAddr gem and is slightly convoluted for emphasis :)
class NetworkResource < ActiveRecord::Base composed_of :cidr, :class_name => 'NetAddr::CIDR', :mapping => [ %w(network_address network), %w(cidr_range bits) ], :allow_nil => true, :constructor => Proc.new { |network_address, cidr_range| NetAddr::CIDR.create("#{network_address}/#{cidr_range}") }, :converter => Proc.new { |value| NetAddr::CIDR.create(value.is_a?(Array) ? value.join('/') : value) } end # Calls the :constructor proc n = NetworkResource.new(:network_address => '192.168.0.1', :cidr_range => 24) # Calls the :converter proc n.cidr = [ '192.168.2.1', 8 ] n.cidr = '192.168.0.1/24' # Doesn't call the :converter proc as the class matches the aggregate class n.cidr = NetAddr::CIDR.create('192.168.2.1/8') # Save and reload - uses the :constructor proc on reload n.save n.reload -
Rob Anderton September 10th, 2008 @ 01:04 PM
Given that this patch fixes what is really a bug in ActiveRecord, doesn’t break existing code and is now the third such patch to address the issue of constructors in composed_of (the other two of course not being applied), is there a good reason not to apply it now?
I appreciate the efforts of Eloy to provide a complete replacement for composed_of and, while I don’t necessarily agree with the syntax, I think this is something that should be investigated further using plugins for now with a view to incorporating the ‘best’ approach into Rails at some point in the future.
But in the interim can the two or three of us who want to use composed_of with classes that don’t conform to its narrow view of how objects should be constructed, actually be allowed to?
-
Michael Koziarski September 10th, 2008 @ 02:53 PM
- Assigned user set to “Michael Koziarski”
- Milestone cleared.
OK, I think I agree, it's probably a good idea to apply this.
Could you flesh out the documentation with a few more examples though.
-
Michael Koziarski September 10th, 2008 @ 02:57 PM
Basically if you turn that comment above into rdoc, I think we're good
-
Rob Anderton September 10th, 2008 @ 05:20 PM
That's great news, thanks!
I've attached an updated patch which includes the original changes plus the example from above and some additional explanations for how things work that hopefully make things a bit clearer.
-
Repository September 10th, 2008 @ 05:50 PM
- State changed from “new” to “committed”
(from [b518b6c0d3e7796e303c2396de97a8d901aeb308]) Expanded documentation for new composed_of options
Signed-off-by: Michael Koziarski michael@koziarski.com [#892 state:committed] http://github.com/rails/rails/co...
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Michael Koziarski
Repository
Rob Anderton
Attachments
Referenced by
-
892
composed_of constructor and converter options
Signed-off-by: Michael Koziarski michael@koziarski.com [#...