class_attribute is too dangerous, here's an alternative

rubyonrails-core
#1

According to the docs, class_attribute does implement inheritance for
class instance variables. However, it doesn't work as it is intended
to, at least if you use "mutual structures", like Hash.

Base.foo = {}
Subclass.foo[:bar] = "bar"

Base.foo # => {:bar => "bar"}

I'm sorry, but this is WRONG and dangerous. The docs tell me to "use
setters" here - not sure how this is supposed to work.

Another "solution" is to initialize the ivar in the subclass, again,
as done here: https://github.com/rails/rails/commit/09195f10bd3

SO, if I need to initialize the ivar, I don't need inheritance. Why is
that called "inheritance" at all?

By accident I solved this with a 10-liner months ago, why not use
something like that? https://github.com/apotonick/hooks/blob/master/lib/hooks/inheritable_attribute.rb

It's simple, clean and does exactly what you expect.

I just found out class_attribute nearly broke my code, so all I want
is prevent people from stepping into that trap, too :smiley:

Cheers!

#2

I like Nick's implementation, not everyone know or remembers that
modyfing object like Hash or Array will also change the superclass
(which is clearly visible in the commit that Nick's linked - that was
exactly the reason of the bug that I was fixing).

I think that it would be nice to have inheritable class attributes
that allows to forget about that issue.

#3

I'm sorry, but this is WRONG and dangerous. The docs tell me to "use
setters" here - not sure how this is supposed to work.

No offense meant - just like "somewhat less than perfect" :wink:

#4

According to the docs, class_attribute does implement inheritance for
class instance variables. However, it doesn't work as it is intended
to, at least if you use "mutual structures", like Hash.

Base.foo = {}
Subclass.foo[:bar] = "bar"

Base.foo # => {:bar => "bar"}

I'm sorry, but this is WRONG and dangerous.

This particular macro does not implement cloning. Not doing so is not
wrong or right a priori, it is just the way it works.

The docs tell me to "use
setters" here - not sure how this is supposed to work.

Have you read the explanation in the AS core extensions guide?

    http://guides.rubyonrails.org/active_support_core_extensions.html#class-attributes

SO, if I need to initialize the ivar, I don't need inheritance. Why is
that called "inheritance" at all?

Well, because they are inherited in the sense that if A < B and B has
a class attribute "x", then A also has "x" and it has the same value
(the same object reference) than B by default.

They are different from ordinary Ruby class variables in that you can
*override* them at any level in the hierarchy. They also provide
access at the instance level.

Please have a look also to the section about class inheritable attributes.

#5

Please have a look also to the section about class inheritable attributes.

Do you basically say what I want is class_inheritable_hash ?

#6

Well, as long as "what you expect" doesn't involve keeping anything more complicated than a flat Hash/Array/etc in the class variable:

irb --> a = { :key_1 => 1, :key_2 => ['x','y'] }
    ==> {:key_1=>1, :key_2=>["x", "y"]}

irb --> b = a.clone
    ==> {:key_1=>1, :key_2=>["x", "y"]}

irb --> b[:key_2] << 'z'
    ==> ["x", "y", "z"]

irb --> a
    ==> {:key_1=>1, :key_2=>["x", "y", "z"]}

Object#clone solves the problems caused when you add a key to hash b in this example, but it doesn't completely separate the objects. This code does:

totally_cloned = Marshal.load(Marshal.dump(source))

but that's fairly inefficient and fails disastrously if an object that can't be marshalled is involved (Procs, for instance).

I can't really decide whether doing the clone causes results that are more or less surprising than the existing version.

--Matt Jones

#7

> By accident I solved this with a 10-liner months ago, why not use
> something like that?https://github.com/apotonick/hooks/blob/master/lib/hooks/inheritable_…

> It's simple, clean and does exactly what you expect.

Well, as long as "what you expect" doesn't involve keeping anything more complicated than a flat Hash/Array/etc in the class variable:

Ouuuuuuuuuuuuuuuuuuuuuch - that's absolutely right. So worst case is
that in my solution you might still operate on a superclass object.
Let me think about that!

#8

Ouuuuuuuuuuuuuuuuuuuuuch - that's absolutely right. So worst case is
that in my solution you might still operate on a superclass object.
Let me think about that!

We have several different options for this depending on the semantics
you're after for your 'class variable things'.

https://github.com/rails/rails/blob/master/activesupport/lib/active_support/core_ext/class/delegating_attributes.rb

Does something closer to what you have in mind, but you still will
have child classes able to alter the parent's Hashes.

The problem is that you're expecting class_attribute to magically
clone itself because you're storing a Hash there. However if instead
of a Hash you were storing something else, say a database connection,
then your solution will be confusing and cause strange errors.

#9

Hi Nick,

According to the docs, class_attribute does implement inheritance for
class instance variables. However, it doesn't work as it is intended
to, at least if you use "mutual structures", like Hash.

Base.foo = {}
Subclass.foo[:bar] = "bar"

Base.foo # => {:bar => "bar"}

I'm sorry, but this is WRONG and dangerous. The docs tell me to "use
setters" here - not sure how this is supposed to work.

Another "solution" is to initialize the ivar in the subclass, again,
as done here: https://github.com/rails/rails/commit/09195f10bd3

SO, if I need to initialize the ivar, I don't need inheritance. Why is
that called "inheritance" at all?

By accident I solved this with a 10-liner months ago, why not use
something like that? https://github.com/apotonick/hooks/blob/master/lib/hooks/inheritable_attribute.rb

It's simple, clean and does exactly what you expect.

I just found out class_attribute nearly broke my code, so all I want
is prevent people from stepping into that trap, too :smiley:

Cheers!

# To modify an inherited hash in-place, each subclass needs its own
copy up-front. Use class_inheritable_attribute for this
copy-on-inherit behavior.

class Base
  class_inheritable_attribute :options
  self.options = {}
end

class Subclass < Base
  self.options[:foo] = 'bar'
end

# Use class_attribute for inheritance that behaves just like Ruby
methods. The superclass value is used unless it's overridden by a
subclass' value.

class Base
  class_attribute :options
  self.options = {}
end

class Subclass < Base
  self.options = self.options.merge(:foo => 'bar')
end

# With a declarative API

class Base
  class_attribute :options
  def self.config(options) self.options = (self.options ||
{}).merge(options) end
end

class Subclass < Base
  config :foo => 'bar'
end

Jeremy