make setter private

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.

Please help.

Thanks.

Hi --

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.

Thanks.

LBD

Hi --

Hi --

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 :slight_smile: 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?

David

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?

Thanks.

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:

Why not override x= then rather than inventing a new set_x method ?

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?

The setter methods get generated on the fly for you (the default ones
just end up calling write_attribute) as David explained - you can't
mark them as private or override and use super to call the original
implementation because the method doesn't exist yet.

Fred

Hi --

Hi --

Hi --

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 :slight_smile: 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.

David

Hi --

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:

Why not override x= then rather than inventing a new set_x method ?

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?

The setter methods get generated on the fly for you (the default ones
just end up calling write_attribute) as David explained - you can't
mark them as private or override and use super to call the original
implementation because the method doesn't exist yet.

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.

David

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

Fred

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

Hi --

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.

David

Thanks everyone for your responses. Felix, your before_save
suggestion sounds promising. But will before_save be able to skip the
save operation if the "do some stuff" fails?

What I want to do is this: always do "some stuff" whenever the x
attribute is updated. But if "some stuff" returns nil which means it
has failed, do not update x, and then also return nil to the call to
update x to let the caller know that the update operation of x has
failed.

I looked up before_save on the API guide and online but still don't
know the answer.

Right now the way I am implementing this is:

def set_x(val)
   catch :some_stuff_has_failed do
     do_some_stuff # throw :some_stuff_has_failed if it fails

     self.x = val
     save!
   end
end

I try to make the setter "x=" private so that anybody using my class
has to use "set_x" to set a value for x. However, I am getting the
"attempt to call private method" error which causes me to post this
thread.

Please let me know if my description of the problem is still
incomplete.

Thanks all for your help.

Thanks everyone for your responses. Felix, your before_save
suggestion sounds promising. But will before_save be able to skip the
save operation if the "do some stuff" fails?

What I want to do is this: always do "some stuff" whenever the x
attribute is updated. But if "some stuff" returns nil which means it
has failed, do not update x, and then also return nil to the call to
update x to let the caller know that the update operation of x has
failed.

I looked up before_save on the API guide and online but still don't
know the answer.

Right now the way I am implementing this is:

def set_x(val)
  catch :some_stuff_has_failed do
    do_some_stuff # throw :some_stuff_has_failed if it fails

    self.x = val
    save!
  end
end

I try to make the setter "x=" private so that anybody using my class
has to use "set_x" to set a value for x. However, I am getting the
"attempt to call private method" error which causes me to post this
thread.

Please let me know if my description of the problem is still
incomplete.

Thanks all for your help.

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 :slight_smile:

HTH,

Felix

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 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.

Thanks everyone. You all have been so helpful. I am very appreciative of this community.

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 :slight_smile:

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