I'm learning ruby and my opinion about attr_reader
is bad. Because you can change the value of particular instance variable from outside of class. For example:
class X
attr_reader :some_string
def initialize
@some_string = "abc"
end
end
tmp = X.new
puts tmp.some_string
tmp.some_string[0] = "aaaaaaaaa"
puts tmp.some_string
When you run this code you can see that the instance variable some_string
has changed. So to avoid this I'm always making my own getter, and returning frozen
object. For example:
class X
def initialize
@some_string = "abc"
end
def some_string
@some_string.freeze
end
end
tmp = X.new
puts tmp.some_string
tmp.some_string[0] = "aaaaaaaaa"
puts tmp.some_string
Now when you run the code, it throws an error saying can't modify frozen String: "abc"
, which is what I wanted. So my question is should I use attr_reader
and is always returning frozen
objects from getters bad practice?
CodePudding user response:
attr_reader
is not "bad", it simply does what it does, which is to return the value of the instance variable, which is a reference. Unlike attr_writer
or attr_accessor
it will not allow you to change the value of the instance variable (ie you can't change what the instance variable refers to)
The question is do you really want or need the reference. For example say you want to convert the value of some_string to upper case. You could use attr_reader
to get the reference to some_string
and then call upcase! on it like below. But having the reference allows you to call any method on the object including the []= method which maybe you don't want to allow. Note: []= is a method that manipulates the content of what some_string references it does not set @some_string to a new value, it still points to the same object, but the object it points to was manipulated.
class Foo
attr_reader :some_string
def initialize()
@some_string = "abc"
end
end
puts "Foo"
foo = Foo.new
some_string = foo.some_string
puts some_string #=> abc
some_string.upcase!
p foo # => #<Foo:0x0000563c38391ac8 @some_string="ABC">
puts some_string.object_id # => 60
some_string[0] = "x"
p foo # => #<Foo:0x0000563c38391ac8 @some_string="xBC">
puts some_string.object_id # => 60 ... ie same object different content
# foo.some_string = "ABC" is a runtime error
If you don't want to allow arbitrary methods to be called on an instance variable then you should not expose it using attr_reader
, rather you should manipulate the instance variable via methods in your class. For example below I "delegate" the upcase! method to the instance variable @some_string, and I provide a string_value method to return a new string with the same value as the instance variable.
class Bar
def initialize()
@some_string = "abc"
end
def upcase!()
@some_string.upcase!
end
def string_value()
"#{@some_string}"
end
end
puts "Bar"
bar = Bar.new
p bar # => #<Bar:0x0000563c383915a0 @some_string="abc">
bar.upcase!
p bar # => #<Bar:0x0000563c383915a0 @some_string="ABC">
some_string = bar.string_value
p some_string # => "ABC"
some_string[0] = "x"
p bar # => #<Bar:0x0000563c383915a0 @some_string="ABC">
p some_string # => "xBC"
So I would say attr_reader
is not bad, you might argue it is over used, I know I will often use it to "get" an instance variable when all I really need is some property on the instance variable.
CodePudding user response:
A lot of developers try to use private attr_reader and use it inside the class or avoid using it at all A good conversations about attr_reader are here and here