This project is archived and is in readonly mode.

composed_of constructor and converter options

#892

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

Reported by Rob Anderton · August 24th, 2008 @ 11:08 AM

Activity

Michael Koziarski
I don't quite follow what :converter is compared to :constructor

August 26th, 2008 @ 05:01 PM

Rob Anderton

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

August 26th, 2008 @ 07:20 PM

Rob Anderton

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?

September 10th, 2008 @ 01:04 PM
Michael Koziarski
- 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.

September 10th, 2008 @ 02:53 PM
Michael Koziarski
Basically if you turn that comment above into rdoc, I think we're good

September 10th, 2008 @ 02:57 PM
Rob Anderton
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.
- improved_composed_of_with_documentation.diff 20.7 KB
September 10th, 2008 @ 05:20 PM
Repository
- State changed from new to committed
(from [b518b6c0d3e7796e303c2396de97a8d901aeb308]) Expanded documentation for new composed_of options

Signed-off-by: Michael Koziarski michael@koziarski.com [#892 composed_of constructor and converter options state:committed] http://github.com/rails/rails/co...

September 10th, 2008 @ 05:50 PM