Skip to content

Commit

Permalink
Adding methods with a generic name to a core Ruby class (and therefore
Browse files Browse the repository at this point in the history
all instances of that class) is not a good idea.

Crack was defining [1] `attributes` accessor methods on the core Ruby
`String` class which in turn caused a very difficult to track down bug
when using Mongoid, because of this [2]. This commit changes Crack so it
just defines the `attributes` accessor methods on the relevant instances
of `String`.

[1] https://github.com/jnunemaker/crack/blob/master/lib/crack/xml.rb#L85
[2] https://github.com/mongoid/mongoid/blob/master/lib/mongoid/relations/builder.rb#L38

Created by floehopper
  • Loading branch information
rubiii committed Apr 30, 2011
1 parent 4c17e75 commit 0fc6651
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
8 changes: 6 additions & 2 deletions lib/nori/xml_utility_node.rb
Expand Up @@ -83,8 +83,12 @@ class << f

if @text
t = typecast_value( unnormalize_xml_entities( inner_html ) )
t.class.send(:attr_accessor, :attributes)
t.attributes = attributes
if t.is_a?(String)
class << t
attr_accessor :attributes
end
t.attributes = attributes
end
return { name => t }
else
#change repeating groups into an array
Expand Down
10 changes: 10 additions & 0 deletions spec/nori/nori_spec.rb
Expand Up @@ -104,6 +104,16 @@
it "default attributes to empty hash if not present" do
@data['opt']['user'][1].attributes.should == {}
end

it "add 'attributes' accessor methods to parsed instances of String" do
@data['opt']['user'][0].should respond_to(:attributes)
@data['opt']['user'][0].should respond_to(:attributes=)
end

it "not add 'attributes' accessor methods to all instances of String" do
"some-string".should_not respond_to(:attributes)
"some-string".should_not respond_to(:attributes=)
end
end

it "should typecast an integer" do
Expand Down

0 comments on commit 0fc6651

Please sign in to comment.