Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Switch from class_inheritable_accessor to class_attribute #9

Merged
merged 1 commit into from

2 participants

@brynary

class_inheritable_accessor is deprecated in Rails 3.1. class_attribute has slightly different semantics, but this change should be functionality neutral since it appears there's only one place where the attachment_names Set is modified.

@brynary brynary Switch from class_inheritable_accessor to class_attribute
* class_inheritable_accessor is deprecated in Rails 3.1
7e6a49c
@bkeepers bkeepers commented on the diff
lib/joint/class_methods.rb
@@ -8,7 +8,7 @@ module Joint
options.symbolize_keys!
name = name.to_sym
- self.attachment_names << name
+ self.attachment_names = attachment_names.dup.add(name)

What's the advantage of duping here? Do we need a test for it?

@brynary
brynary added a note

Duping is required so we don't mutate the Set object. This is required, and if you remove the dup two tests will fail so it seems to be well covered.

Docs on class_attribute explain this a bit: api.rubyonrails.org/classes/Class.html#method-i-class_attribute

Specifically: "However, you need to be aware when using class_attribute with mutable structures as Array or Hash. In such cases, you don’t want to do changes in places but use setters:"

awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bkeepers bkeepers merged commit f7a0609 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 23, 2011
  1. @brynary

    Switch from class_inheritable_accessor to class_attribute

    brynary authored
    * class_inheritable_accessor is deprecated in Rails 3.1
This page is out of date. Refresh to see the latest.
Showing with 2 additions and 2 deletions.
  1. +1 −1  lib/joint.rb
  2. +1 −1  lib/joint/class_methods.rb
View
2  lib/joint.rb
@@ -6,7 +6,7 @@ module Joint
extend ActiveSupport::Concern
included do
- class_inheritable_accessor :attachment_names
+ class_attribute :attachment_names
self.attachment_names = Set.new
include attachment_accessor_module
end
View
2  lib/joint/class_methods.rb
@@ -8,7 +8,7 @@ def attachment(name, options = {})
options.symbolize_keys!
name = name.to_sym
- self.attachment_names << name
+ self.attachment_names = attachment_names.dup.add(name)

What's the advantage of duping here? Do we need a test for it?

@brynary
brynary added a note

Duping is required so we don't mutate the Set object. This is required, and if you remove the dup two tests will fail so it seems to be well covered.

Docs on class_attribute explain this a bit: api.rubyonrails.org/classes/Class.html#method-i-class_attribute

Specifically: "However, you need to be aware when using class_attribute with mutable structures as Array or Hash. In such cases, you don’t want to do changes in places but use setters:"

awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
after_save :save_attachments
after_save :destroy_nil_attachments
Something went wrong with that request. Please try again.