Permalink
Browse files

heavily rework Dash for bugfixes and performance

 - optimize `properties` and `defaults` so they don't do a full
   ancestry lookup each time a property is accessed

 - API change: `properties` is now a Set of symbols (was Array of strings)

 - API change: `defaults` is a Hash with symbol keys and doesn't include
   properties without defaults

 - optimize generated accessors to skip lookup if a property exists

 - fixed accessing the dash with square brackets

 - change keys to be stored internally as strings instead of symbols

 - change initialization so it doesn't initialize properties without
   defaults to nil

 - allow creating a dash with a block that defines default values as
   fallback (regular Hash behavior)

 - allow redefining a property in descendants in order to set a new
   default value or clear the existing one

 - rewritten test suite to be more concise and better focused on what's important
  • Loading branch information...
1 parent 309ddd4 commit 8d4e599209da9827c5748946b843f6c72220785d @mislav committed Aug 25, 2010
Showing with 151 additions and 112 deletions.
  1. +46 −44 lib/hashie/dash.rb
  2. +105 −68 spec/hashie/dash_spec.rb
View
@@ -1,7 +1,7 @@
require 'hashie/hash'
+require 'set'
module Hashie
- include Hashie::PrettyInspect
# A Dash is a 'defined' or 'discrete' Hash, that is, a Hash
# that has a set of defined keys that are accessible (with
# optional defaults) and only those keys may be set or read.
@@ -26,83 +26,85 @@ class Dash < Hashie::Hash
def self.property(property_name, options = {})
property_name = property_name.to_sym
- (@properties ||= []) << property_name
- (@defaults ||= {})[property_name] = options.delete(:default)
+ self.properties << property_name
- class_eval <<-RUBY
- def #{property_name}
- self[:#{property_name}]
- end
+ if options[:default] or self.defaults[property_name]
+ self.defaults[property_name] = options[:default]
+ end
- def #{property_name}=(val)
- self[:#{property_name}] = val
- end
- RUBY
- end
+ unless instance_methods.map { |m| m.to_s }.include?("#{property_name}=")
+ class_eval <<-ACCESSORS
+ def #{property_name}
+ _regular_reader(#{property_name.to_s.inspect})
+ end
- # Get a String array of the currently defined
- # properties on this Dash.
- def self.properties
- properties = []
- ancestors.each do |elder|
- if elder.instance_variable_defined?("@properties")
- properties << elder.instance_variable_get("@properties")
- end
+ def #{property_name}=(value)
+ _regular_writer(#{property_name.to_s.inspect}, value)
+ end
+ ACCESSORS
end
- properties.flatten.map{|p| p.to_s}
+ if defined? @subclasses
+ @subclasses.each { |klass| klass.property(property_name, options) }
+ end
end
- # Check to see if the specified property has already been
- # defined.
- def self.property?(prop)
- properties.include?(prop.to_s)
+ class << self
+ attr_reader :properties, :defaults
end
+ instance_variable_set('@properties', Set.new)
+ instance_variable_set('@defaults', {})
- # The default values that have been set for this Dash
- def self.defaults
- properties = {}
- ancestors.each do |elder|
- if elder.instance_variable_defined?("@defaults")
- properties.merge! elder.instance_variable_get("@defaults")
- end
- end
+ def self.inherited(klass)
+ (@subclasses ||= Set.new) << klass
+ klass.instance_variable_set('@properties', self.properties.dup)
+ klass.instance_variable_set('@defaults', self.defaults.dup)
+ end
- properties
+ # Check to see if the specified property has already been
+ # defined.
+ def self.property?(name)
+ properties.include? name.to_sym
end
# You may initialize a Dash with an attributes hash
# just like you would many other kinds of data objects.
- def initialize(attributes = {})
- self.class.properties.each do |prop|
- self.send("#{prop}=", self.class.defaults[prop.to_sym])
+ def initialize(attributes = {}, &block)
+ super(&block)
+
+ self.class.defaults.each_pair do |prop, value|
+ self.send("#{prop}=", value)
end
attributes.each_pair do |att, value|
self.send("#{att}=", value)
end if attributes
end
+ alias_method :_regular_reader, :[]
+ alias_method :_regular_writer, :[]=
+ private :_regular_reader, :_regular_writer
+
# Retrieve a value from the Dash (will return the
# property's default value if it hasn't been set).
def [](property)
- super(property.to_sym) if property_exists? property
+ assert_property_exists! property
+ super(property.to_s)
end
# Set a value on the Dash in a Hash-like way. Only works
# on pre-existing properties.
def []=(property, value)
- super if property_exists? property
+ assert_property_exists! property
+ super(property.to_s, value)
end
private
- # Raises an NoMethodError if the property doesn't exist
- #
- def property_exists?(property)
- unless self.class.property?(property.to_sym)
+
+ def assert_property_exists!(property)
+ unless self.class.property?(property)
raise NoMethodError, "The property '#{property}' is not defined for this Dash."
end
- true
end
end
end
View
@@ -10,100 +10,137 @@ class Subclassed < DashTest
property :last_name
end
-describe Hashie::Dash do
- it 'should be a subclass of Hashie::Hash' do
- (Hashie::Dash < Hash).should be_true
+describe DashTest do
+
+ it('subclasses Hashie::Hash') { should respond_to(:to_mash) }
+
+ its(:to_s) { should == '<#DashTest count=0>' }
+
+ it 'lists all set properties in inspect' do
+ subject.first_name = 'Bob'
+ subject.email = 'bob@example.com'
+ subject.inspect.should == '<#DashTest count=0 email="bob@example.com" first_name="Bob">'
end
-
- it '#inspect should be ok!' do
- dash = DashTest.new
- dash.email = "abd@abc.com"
- dash.inspect.should == "<#DashTest count=0 email=\"abd@abc.com\" first_name=nil>"
+
+ its(:count) { should be_zero }
+
+ it { should respond_to(:first_name) }
+ it { should respond_to(:first_name=) }
+ it { should_not respond_to(:nonexistent) }
+
+ it 'errors out for a non-existent property' do
+ lambda { subject['nonexistent'] }.should raise_error(NoMethodError)
end
- describe ' creating properties' do
- it 'should add the property to the list' do
- DashTest.property :not_an_att
- DashTest.properties.include?('not_an_att').should be_true
+ context 'writing to properties' do
+ it 'fails writing to a non-existent property using []=' do
+ lambda { subject['nonexistent'] = 123 }.should raise_error(NoMethodError)
end
- it 'should create a method for reading the property' do
- DashTest.new.respond_to?(:first_name).should be_true
+ it 'works for an existing property using []=' do
+ subject['first_name'] = 'Bob'
+ subject['first_name'].should == 'Bob'
+ subject[:first_name].should == 'Bob'
end
- it 'should create a method for writing the property' do
- DashTest.new.respond_to?(:first_name=).should be_true
+ it 'works for an existing property using a method call' do
+ subject.first_name = 'Franklin'
+ subject.first_name.should == 'Franklin'
end
end
- describe 'reading properties' do
- it 'should raise an error when reading a non-existent property' do
- lambda{@dash['abc']}.should raise_error(NoMethodError)
+ describe '.new' do
+ it 'fails with non-existent properties' do
+ lambda { described_class.new(:bork => '') }.should raise_error(NoMethodError)
end
- end
- describe ' writing to properties' do
- before do
- @dash = DashTest.new
+ it 'should set properties that it is able to' do
+ obj = described_class.new :first_name => 'Michael'
+ obj.first_name.should == 'Michael'
end
-
- it 'should not be able to write to a non-existent property using []=' do
- lambda{@dash['abc'] = 123}.should raise_error(NoMethodError)
+
+ it 'accepts nil' do
+ lambda { described_class.new(nil) }.should_not raise_error
end
-
- it 'should be able to write to an existing property using []=' do
- lambda{@dash['first_name'] = 'Bob'}.should_not raise_error
- end
-
- it 'should be able to read/write to an existing property using a method call' do
- @dash.first_name = 'Franklin'
- @dash.first_name.should == 'Franklin'
+
+ it 'accepts block to define a global default' do
+ obj = described_class.new { |hash, key| key.to_s.upcase }
+ obj.first_name.should == 'FIRST_NAME'
+ obj.count.should be_zero
end
end
-
- describe ' initializing with a Hash' do
- it 'should not be able to initialize non-existent properties' do
- lambda{DashTest.new(:bork => 'abc')}.should raise_error(NoMethodError)
- end
-
- it 'should set properties that it is able to' do
- DashTest.new(:first_name => 'Michael').first_name.should == 'Michael'
+
+ describe 'properties' do
+ it 'lists defined properties' do
+ described_class.properties.should == Set.new([:first_name, :email, :count])
end
- end
-
- describe 'initializing with a nil' do
- it 'accepts nil' do
- lambda { DashTest.new(nil) }.should_not raise_error
+
+ it 'checks if a property exists' do
+ described_class.property?('first_name').should be_true
+ described_class.property?(:first_name).should be_true
end
- end
-
- describe ' defaults' do
- before do
- @dash = DashTest.new
+
+ it 'doesnt include property from subclass' do
+ described_class.property?(:last_name).should be_false
end
-
- it 'should return the default value for defaulted' do
- DashTest.property :defaulted, :default => 'abc'
- DashTest.new.defaulted.should == 'abc'
+
+ it 'lists declared defaults' do
+ described_class.defaults.should == { :count => 0 }
end
end
end
-describe Subclassed do
- it "should inherit all properties from DashTest" do
- Subclassed.properties.size.should == 6
+describe Hashie::Dash, 'inheritance' do
+ before do
+ @top = Class.new(Hashie::Dash)
+ @middle = Class.new(@top)
+ @bottom = Class.new(@middle)
end
-
- it "should inherit all defaults from DashTest" do
- Subclassed.defaults.size.should == 6
+
+ it 'reports empty properties when nothing defined' do
+ @top.properties.should be_empty
+ @top.defaults.should be_empty
end
-
- it "should init without raising" do
- lambda { Subclassed.new }.should_not raise_error
- lambda { Subclassed.new(:first_name => 'Michael') }.should_not raise_error
+
+ it 'inherits properties downwards' do
+ @top.property :echo
+ @middle.properties.should include(:echo)
+ @bottom.properties.should include(:echo)
+ end
+
+ it 'doesnt inherit properties upwards' do
+ @middle.property :echo
+ @top.properties.should_not include(:echo)
+ @bottom.properties.should include(:echo)
+ end
+
+ it 'allows overriding a default on an existing property' do
+ @top.property :echo
+ @middle.property :echo, :default => 123
+ @bottom.properties.to_a.should == [:echo]
+ @bottom.new.echo.should == 123
end
+
+ it 'allows clearing an existing default' do
+ @top.property :echo
+ @middle.property :echo, :default => 123
+ @bottom.property :echo
+ @bottom.properties.to_a.should == [:echo]
+ @bottom.new.echo.should be_nil
+ end
+end
- it "should share defaults from DashTest" do
- Subclassed.new.count.should == 0
+describe Subclassed do
+
+ its(:count) { should be_zero }
+
+ it { should respond_to(:first_name) }
+ it { should respond_to(:first_name=) }
+ it { should respond_to(:last_name) }
+ it { should respond_to(:last_name=) }
+
+ it 'has one additional property' do
+ described_class.property?(:last_name).should be_true
end
+
end

0 comments on commit 8d4e599

Please sign in to comment.