There's not quite enough information in the code you've got here to
see what's going on. Do you have a superclass with a definition for
the attribute_A= method? (I'm trying to figure out why you're calling
super.)
David
--
David A. Black / Ruby Power and Light, LLC
Ruby/Rails consulting & training: http://www.rubypal.com
Now available: The Well-Grounded Rubyist (http://manning.com/black2)
Training! Intro to Ruby, with Black & Kastner, September 14-17
(More info: http://rubyurl.com/vmzN)
On Sun, 19 Jul 2009, Learn By Doing wrote:
>
> On Jul 19, 6:39 pm, "David A. Black" <dbl...@rubypal.com> wrote:
>> Hi --
>>
>>
>>
>> On Sun, 19 Jul 2009, Ease Bus wrote:
>>> Hi,
>>
>>> I would like to rename the setter of "attribute_A" with the following:
>>
>>> def set_attribute_A(val)
>>> self.attribute_A = val
>>> end
>>
>>> private
>>
>>> def attribute_A=(val)
>>> super
>>> end
>>
>>> But then I get "NoMethodError: Attempt to call private method" I thought that a private method is
>>> accessible from within the same instance.
>>
>> There's not quite enough information in the code you've got here to
>> see what's going on. Do you have a superclass with a definition for
>> the attribute_A= method? (I'm trying to figure out why you're calling
>> super.)
>>
>
> Hi David,
>
> Thanks for your response. attribute_A is just an attribute in a model
> that is a subclass of ActiveRecord::Base. attribute_A is of type
> integer.
>
> The reason I used "super" is because I just wanted to assign the value
> of "val" to attribute_A in the private setter method "attribute_A=" I
> can't say self.attribute_A = val because that is calling the private
> method I am defining.
I'm trying to duplicate the error message you're getting, but I can't
quite get my example code to do that. On the other hand, I believe
there are other issues.... If you define, say, x= to call super, the
problem is that you'll hit method_missing (because there's no built-in
x= method). And method_missing will obligingly define x= for you, and
not make it private. That's just a result of how AR implements
method_missing; it does some on-the-spot definition of attribute
methods.
I suspect you're going to have trouble with this whole thing, because
you're going up against AR's internals. I may well be overlooking
something, but that's my (late at night :-) impression. I can't help
wondering whether you absolutely need to do this. The whole "set_x"
naming scheme is very unidiomatic in Ruby, and the convention of the =
methods acting like assignments, in their return semantics, is
generally not considered a problem.
Is there something in that original goal that you could reassess?
> Hi David,
>
> The reason I need to redefine the assignment and make it private is
> that because whenever x is assigned a new value, some other processes
> must take place:
>
> def set_x(val)
> self.x = val
> # some other processes take place here
> end
>
> private
>
> def x=(val)
> super
> end
>
> I don't understand when you say that there is no built-in method for
> "x=". How else can one assign value to an attribute?
Meaning, there's no x= method in ActiveRecord::Base. It only comes
into being because there's a column of that name in your database.
Another possibility that occurs to me is:
def x=(val)
super
# other stuff here
end
There's something that I can't quite put my finger on that bugs me
about that, though... not sure what it is, and maybe it's fine. (Maybe
it's the fact that it relies on the underlying implementation of
method_missing.) But that takes us back to the original question about
returning an arbitrary value from a = method. That's the thing I'm
wondering whether you really need to do.
You can do this and get, by coincidence, something like the desired
effect:
def x=(val)
super
# do other stuff
end
because the call to super will trigger AR::B#method_missing, but the
more I look at it and play around with it, the more I think it's way
too fragile and too closely couple to the method_missing
implementation to use.
> On Jul 21, 12:02 am, "David A. Black" <dbl...@rubypal.com> wrote:
>> You can do this and get, by coincidence, something like the desired
>> effect:
>>
>> def x=(val)
>> super
>> # do other stuff
>> end
>>
>> because the call to super will trigger AR::B#method_missing, but the
>> more I look at it and play around with it, the more I think it's way
>> too fragile and too closely couple to the method_missing
>> implementation to use.
>
> I haven't actually tried this but it looks like activerecord won't
> generate an accessor if the method already exists (see
> define_attribute_methods in activerecord/lib/active_record/
> attribute_methods.rb
It's a little hard to really help because of the lack of information
regarding the real problem behind the OP's question (such as: why
would you want to return a boolean from a setter method, or what is
the other stuff that goes in the setter, is it really something that
belongs there?), but I think the most sensible thing to do would be to
just:
def x=(val)
# do other stuff
write_attribute(x, val)
end
(off the top of my head, be sure to verify the syntax...)
I think the crucial part here is: does this "other stuff" really
belong in the setter? Shouldn't it go in some before_save callback for
example? Or in an observer? Or maybe even just in another function like:
def do_some_stuff_with_and_set_x(val)
# do other stuff
self.x = val
return whatever
end
My point here being that a setter should really just do that: set an
attribute to a certain value. For any validation of sorts, you have
validation callbacks, for any sanitizing that needs to be done, you
can hook into the before_save callback, and so forth.
Felix
On Tue, 21 Jul 2009, Frederick Cheung wrote:
>
>
> On Jul 21, 12:02 am, "David A. Black" <dbl...@rubypal.com> wrote:
>> You can do this and get, by coincidence, something like the desired
>> effect:
>>
>> def x=(val)
>> super
>> # do other stuff
>> end
>>
>> because the call to super will trigger AR::B#method_missing, but the
>> more I look at it and play around with it, the more I think it's way
>> too fragile and too closely couple to the method_missing
>> implementation to use.
>
> I haven't actually tried this but it looks like activerecord won't
> generate an accessor if the method already exists (see
> define_attribute_methods in activerecord/lib/active_record/
> attribute_methods.rb
It won't, but super triggers method_missing anyway because the method
is defined in the subclass, not in AR::Base itself. So every call to
x= goes back to method_missing, decides it's not really missing,
etc.
There's also something going on with private that I haven't quite
figured out. If I do this:
private
def x=(val)
super
end
etc., then when I do obj.x=("y"), I don't get an error; it just
bypasses the private version of the method and hits
AR::B#method_missing, as if the private method didn't exist. I think.
Anyway -- more than enough reason to avoid the whole thing.
That sounds a lot like a validation, apart from saving the model in
the setter, which you shouldn't do. From what I've gathered till now,
rails assumes you always work on a model as a whole, not on a single
attribute of the model. Another thing that you must always take care
of: If for whatever reason you decide to override any default method
or whatever of any API you use, make sure to always have the same
return value. Rails assumes a setter for an attribute of an
ActiveRecord object returns the attribute, heck even ruby assumes a
setter returns the object being set (or fail) for _whatever_ object:
"""
$ irb
>> x = 3
=> 3
>> x = :some_funky_symbol
=> :some_funky_symbol
>> x = some_variable_not_set
NameError: undefined local variable or method `some_variable_not_set'
for main:Object
from (irb):3
"""
Anyway, if your some_stuff is a validation, i.e. doesn't modify the
value of x, try this:
=== In the model ===
validate :some_validation
private
def some_validation
# do some stuff with self.x
# return true or whatever you want if it validates, return false or
an exception if not
end
=== In the controller ===
@my_model.x = "some_value"
@my_model.valid? # returns false if not all validations pass
@my_model.save # also fails (can't remember if it returns false) if
not all validations pass, saves the model to the db if they do
(you obviously don't need the valid? step, as it is called in save
too, but may this will help you in some way)
This separation between the data (model), and the business logic, i.e.
what you do with the data (controller) is just a consequence of the
strong MVC separation in rails.
One thing that worries me, is that I still don't know if the
some_stuff you want to do with x also changes x. If it does, you
should separate the x processing part from the validation, put the
processing part in the public setter x=(val), and make it return the
processed x value, and put the validation as explained above.
For completeness' sake: if some part of before_save fails, save will
also fail, although I'm not sure if a false return value will work, or
if you need to throw an exception.
In the end, I would urge you to rethink your MVC model: a setter
should return the object that gets to the internal attribute/object, a
setter should never save the object in the process (I'd even go as far
as saying that no function in the model should ever call save or
save!, because in 99% of the cases, it might bite your dog), the
validation of the attributes, be it sanity, existence or compliance to
some format, should occur on save, and the handling of failing
validations or a failing save should happen in the controller, not in
the model (maybe have a look at the defaults in a scaffolded object
and model: the update method in the controller for example fetches the
object, writes the parameters it got in the attributes of the object,
and the does if @the_object.save do some_happy_stuff else be_sad end).
I hope I was clear enough, but if there still is something unclear to
you, feel free to ask :-)
HTH,
Felix
LBD
> Felix,
>
> Wow! Thank you very much for the detail and clear answer. I knew
> there was something wrong when I had to "throw" and "catch." I read
> that such construct is rare. Following your advices, I have
> rewritten the whole thing into validate and before_save, and
> elevates the "save" operation out of the model. The conditional
> statement that I previously had buried in another model which was
> used to "throw" now is in the validation.
>
> I did not expect that the original question would lead me to improve
> a good portion of the code following the Rails good practices.
> Thanks again.
I'm glad I could help :-)
> I still have this nagging question though: How do I prevent an
> attribute from being written to from outside the class? In other
> words, how do I make a setter function private so that only other
> functions in the class can call it.
Well, If you are only concerned about the setter, have a look at
attr_protected and attr_readonly. The first one prevents the attribute
from being mass assigned, i.e. you can do
my_instance.protected_attribute = "something", but not
my_instance.update_attributes :protected_attribute =>
"something", :other_attribute => "something else". The second should
be quite clear, and as one would suspect allows you to set the
attribute when first creating the element, but will silently ignore
any subsequent attempts to update the attribute.
I guess if you really (really really really) wanted to keep someone
from touching an attribute defined by a column of your model (all the
columns in the table for your model get automagically converted to
getters and setters) from outside the mode, you could do something like:
def dont_read_me
raise whatever_exception # or even do nothing
end
def dont_read_me=(val)
val # iirc a setter is assumed to return the value it gets fed, bad
things may happen if it doesn't, or
rais whatever_exception # that would really make sure anyone using
the setter noticed you don't want touching the attribute
end
and you'ld have to use read_attribute and write_attribute in the rest
of the model to get to those (and now that I had a look at the API,
I'm not sure those 2 are private, so you'ld have to test it beforehand
anyway...).
Felix