Rails core - Jan 2011 - class_attribute is too dangerous, here's an alternative

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

Cheers!

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

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.

On Jan 21, 9:41 pm, Nick Sutterer <apoton...@gmail.com>
wrote:
> 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_...
>
> 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 :-D
>
> Cheers!
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

> 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" ;-)
> 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_...
>
> 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 :-D
>
> Cheers!
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Fri, Jan 21, 2011 at 9:41 PM, Nick Sutterer <apotonick@gmail.com>
wrote:
> 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.

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

> Please have a look also to the section about class inheritable attributes.
>
Do you basically say what I want is class_inheritable_hash ?

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On Jan 21, 2011, at 3:41 PM, Nick Sutterer wrote:
> 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.
> 
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

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

On 21 Jan., 22:27, Matt Jones <al2o...@gmail.com>
wrote:
> On Jan 21, 2011, at 3:41 PM, Nick Sutterer wrote:
>
> > 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!
> 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
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

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




-- 
Cheers

Koz

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

Hi Nick,

On Fri, Jan 21, 2011 at 12:41 PM, Nick Sutterer <apotonick@gmail.com>
wrote:
> 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 :-D
>
> 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

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.