Skip to content

Commit

Permalink
heavily rework Dash for bugfixes and performance
Browse files Browse the repository at this point in the history
 - 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
mislav committed Aug 26, 2010
1 parent 309ddd4 commit 8d4e599
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 112 deletions.
90 changes: 46 additions & 44 deletions lib/hashie/dash.rb
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
173 changes: 105 additions & 68 deletions spec/hashie/dash_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.