http://github.com/Fingertips/attribute-decorator/tree/master
Manfred
In this case I don't think there's a reason other than consistency
with the associations, but in other parts of active record it's
because sometimes those constants aren't actually ready for use. So
take:
class Bar
belongs_to Foo
def self.some_method
end
end
class Foo
has_many :whatevers, :class=>Bar
end
In the has_many call, Foo.respond_to? :some_method returns false
because that method hasn't been defined yet.
> I would like to make our attribute-decorator plugin use that instead
> of a string which has to be constantized.
> Next to a light performance improvement, this would allow you to
> define a simple decorator wrapper class without any
> magic. For the IPAddr example, if I understood the original example
> correct that is :) :
Your plugin is pretty interesting stuff, it seems it could be a nice
replacement for composed_of at some stage in the near future? Have
you put any thought into turning it into a patch?
--
Cheers
Koz
Ok that makes sense and is basically what I expected.
I'll add the :class option then.
> Your plugin is pretty interesting stuff, it seems it could be a nice
> replacement for composed_of at some stage in the near future? Have
> you put any thought into turning it into a patch?
Thanks.
At the moment we're very busy with an application that uses this plugin.
Once we think it has matured enough we'd love to turn this into a patch,
this is why we wrote it as a plugin with lots of docs etc to begin
with :)
This would be in the not too distant future and if enough people are
interested in said patch.
Eloy
Eloy
While in Berlin for RailsConf this week, we'll probably have a stab at
creating a patch out of this.
I think that discussing this at length will in the end bring us nowhere,
so it might be a better idea if you would create your own implementation
so we have something real to compare and discuss. Which is especially
important for the core team.
Having said that I will try to respond to each of your concerns as
good as possible about what our ideas are :)
> 1. The anonymous class approach as shown in this thread for the IPAddr
> example is nice in that it does mean I don't need to create a special
> class just for aggregation, however I don't like the idea of forcing
> methods (parse, to_a and optionally valid?) into a class to make this
> work.
Well actually we are in fact defining a new wrapper class with
Class.new,
which would probably be done under the hood anyway if we would take
for instance a block for attribute_decorator.
My main concern is that the decorator layer shouldn't try to be to
smart and thus add bloat
code for things than can just as easily be handled by the existing
standard Ruby tools.
Like Class.new with a block and using ducktyping conventions like to_a
and to_s (which I failed to include in the IPAddr example...).
Also I would probably only use this Class.new way in the most simple
cases,
because in more typical cases I'd like to have the logic nicely tugged
away in it's
own class allowing better testability.
> Also what happens if a class already has these methods, they
> don't provide the behaviour needed for attribute decoration but are
> needed elsewhere in the code?
Well the example I gave where I subclassed IPAddr was just one way of
doing it.
The beauty in not assuming too much, is that you as the developer are
free
to solve it in any way as you see fit.
Back to your example. If the class that's being wrapped already has
these methods
defined, you can either use super, as is actually shown in the IPAddr
example,
or write a proxy class which delegates to the IPAddr class in the
correct manner.
Both are IMO very sane alternatives, as is with any other Ruby class.
In other words, there shouldn't be anything new to people in doing it
this way,
because it's all plain Ruby that they could/should already know.
Something which I find absolutely not to be the case with convoluted
methods like composed_of, which try to do everything but then fail to
do the
one thing they should do in a concise, but more importantly, readable
manner.
> 2. The use of a special class for aggregation, as in the CompositeDate
> example from the source code, is good for DRYness as I can write my
> parse and to_a methods in one place and all models that use the same
> aggregations can use the same code (as an aside you could of course
> achieve something similar by putting specialised versions of
> composed_of in a module).
More importantly, it separates the code more and makes it better
testable like
any other Ruby class. Which would be harder when you need a model
context for it to work, because the aggregation logic is part of the
model
instead of it's own independent class.
> However it doesn't feel right that my
> aggregated attribute is now an instance of this class: again using the
> CompositeDate example, I'd like the date_of_birth attribute to be a
> Date object, not a CompositeDate.
Well, like shown in the IPAddr example, you could make CompositeDate a
subclass of Date
and add your specific logic there, it should then still walk and quack
like a Date instance
and even be an instance_of?(Date). Solve it as you see fit for each
specific situation.
> Perhaps a better solution would be to employ some mapping code that
> simply handles conversion/parsing, something which is a little closer
> to the composition generator posted by another Chris near the start of
> this thread. Decorated attributes would then be of the expected class
> and that class would not need to have any new methods grafted on.
Like said before, this is what attribute_decorator already allows you
to do,
we expect you to have implemented the proper methods for conversion/
parsing to work
and the returned instance can be of the expected class, or rather a
subclass, if you wish so.
> Having thought about it, could we not use something like the
> association extension syntax to do this? For example: http://www.pastie.org/263192
The only benefit I see in this approach, is that creating an anonymous
class/module is done
behind the hood instead of doing it yourself. Basically saving you
from having to write: ":class => Class.new do … end"
This shoves the logic for how to create this class into the
aggregation layer, where it doesn't belong.
You will only limit yourself to all available options by doing it this
way.
> Also following the association extension syntax, we could place
> commonly used aggregations into a module and pass an :extend option to
> attribute_decorator like so:
>
> attribute_decorator :date_of_birth, :decorates =>
> [:day, :month, :year], :extend => CompositeDateExtension
Or you could simply define a module which does this when included:
module DateOfBirthDecorator
def self.included(klass)
klass.attribute_decorator :date_of_birth, :decorates =>
[:day, :month, :year], :class => CompositeDate
end
end
Then it would only be a matter of doing:
class Member < ActiveRecord::Base
include DateOfBirthDecorator
end
class Artist < ActiveRecord::Base
include DateOfBirthDecorator
end
Etc.
I don't see a real need to abstract this any further...
It all boils down, for us at least, to the fact that Ruby itself
already provides
us with everything needed in these cases. So the mapping layer
should be as thin and clear as possible, instead of re-inventing the
wheel.
Thus making it easier to debug AND be more flexible at the same time.
> I've used attribute_decorator as the method name here, although it
> could just as easily be composed_of but that would mean breakage for
> the two or three people currently using composed_of :)
>
> I can understand the choice of parse over coerce as previously
> discussed, however I would like to find a better, more descriptive
> name for to_a: I know it *is* turning attributes into an array, but
> that's really an implementation detail, the name doesn't really tell
> us why it is doing that. I was thinking along the lines of mapping or
> map but they're not really working for me either.
> Obviously more thought is needed but what do you think?
I'm not 100% sure it should need to tell us why, because it simply
implements a protocol.
The attributes hash, as you mention in your next email, could be an
alternative
to this. But then again, you need to decide if we should also send a
hash to the initializer,
which will only lengthen your code.
With an array, the protocol is to simply take and return the
attributes in the same order as
specified in the :decorates option, it is a pretty simple convention
IMO.
Cheers,
Eloy
Eloy
> from having to write: ":class => Class.new doend"