Skip to content

Commit

Permalink
Change Virtus::InstanceMethods#initialize to not create an empty hash…
Browse files Browse the repository at this point in the history
… by default

* When no arguments are passed in, the end result is the same as when a nil is
  passed in, so there's no need to create an empty Hash and then "set" the
  internal state with it. Better to just default to nil, and short-circuit the
  call.
* Added specs so that if anything other than nil or an object that responds to
  #to_hash is passed in an exception is raised. Like DM1, by default I *only*
  want constructors handling Hash-like objects or nil; anything else passed in
  should be considered an exceptional case.
  • Loading branch information
dkubb committed Mar 12, 2012
1 parent 1b3e150 commit bc68a3f
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 14 deletions.
4 changes: 2 additions & 2 deletions lib/virtus/instance_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ module InstanceMethods
# @return [undefined]
#
# @api private
def initialize(attributes = {})
self.attributes = attributes if attributes.respond_to?(:to_hash)
def initialize(attributes = nil)
self.attributes = attributes if attributes
end

# Returns a value of the attribute with the given name
Expand Down
34 changes: 22 additions & 12 deletions spec/unit/virtus/instance_methods/initialize_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,35 @@
end
end

let(:subject) do
described_class.new(attributes)
end
context 'with Hash argument' do
subject { described_class.new(:name => name) }

context "with valid argument" do
let(:attributes) do
{:name => 'john'}
end
let(:name) { stub('name') }

it 'sets attributes' do
subject.name.should == attributes[:name]
subject.name.should equal(name)
end
end

context "with 'non-hashy' argument" do
let(:attributes) { '' }
context 'with nil argument' do
subject { described_class.new(nil) }

it 'does not try to set attributes' do
expect { subject }.to_not raise_error(NoMethodError)
it 'does not set attributes' do
subject.name.should be_nil
end
end

context 'with no arguments' do
subject { described_class.new }

it 'does not set attributes' do
subject.name.should be_nil
end
end

context' with argument that does not respond to #to_hash' do
subject { described_class.new(Object.new) }

specify { expect { subject }.to raise_error(NoMethodError) }
end
end

0 comments on commit bc68a3f

Please sign in to comment.