svn commit: r688891 - in /incubator/thrift/trunk/lib/rb: lib/thrift/struct.rb spec/struct_spec.rb

[kc...@apache.org]

Author: kclark
Date: Mon Aug 25 14:34:19 2008
New Revision: 688891

URL: http://svn.apache.org/viewvc?rev=688891&view=rev
Log:
rb: Speed up Struct#initialize for optional fields [THRFIT-112]

Struct#initialize previously walked over every field and checked for
default values before assigning nil. The new approach assigns defaults
only to fields that have defaults, and lets Ruby handle nil ivars.

Author: Bryan Duxbury

Modified:
    incubator/thrift/trunk/lib/rb/lib/thrift/struct.rb
    incubator/thrift/trunk/lib/rb/spec/struct_spec.rb

Modified: incubator/thrift/trunk/lib/rb/lib/thrift/struct.rb
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/lib/thrift/struct.rb?rev=688891&r1=688890&r2=688891&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/lib/thrift/struct.rb (original)
+++ incubator/thrift/trunk/lib/rb/lib/thrift/struct.rb Mon Aug 25 14:34:19 2008
@@ -4,12 +4,51 @@
 module Thrift
   module Struct
     def initialize(d={})
-      each_field do |fid, type, name, default|
-        value = d.delete(name.to_s) { d.delete(name.to_sym) { default.dup rescue default } }
-        Thrift.check_type(value, struct_fields[fid], name) if Thrift.type_checking
+      # get a copy of the default values to work on, removing defaults in favor of arguments
+      fields_with_defaults = fields_with_default_values.dup
+      d.each_key do |name|
+        fields_with_defaults.delete(name.to_s)
+      end
+      
+      # assign all the user-specified arguments
+      d.each do |name, value|
+        unless name_to_id(name.to_s)
+          raise Exception, "Unknown key given to #{self.class}.new: #{name}"
+        end
+        Thrift.check_type(value, struct_fields[name_to_id(name.to_s)], name) if Thrift.type_checking
         instance_variable_set("@#{name}", value)
       end
-      raise Exception, "Unknown keys given to #{self.class}.new: #{d.keys.join(", ")}" unless d.empty?
+      
+      # assign all the default values
+      fields_with_defaults.each do |name, default_value|
+        instance_variable_set("@#{name}", (default_value.dup rescue default_value))
+      end
+    end
+
+    def fields_with_default_values
+      fields_with_default_values = self.class.instance_variable_get("@fields_with_default_values")
+      unless fields_with_default_values
+        fields_with_default_values = {}
+        struct_fields.each do |fid, field_def|
+          if field_def[:default]
+            fields_with_default_values[field_def[:name]] = field_def[:default]
+          end
+        end
+        self.class.instance_variable_set("@fields_with_default_values", fields_with_default_values)
+      end
+      fields_with_default_values
+    end
+
+    def name_to_id(name)
+      names_to_ids = self.class.instance_variable_get("@names_to_ids")
+      unless names_to_ids
+        names_to_ids = {}
+        struct_fields.each do |fid, field_def|
+          names_to_ids[field_def[:name]] = fid
+        end
+        self.class.instance_variable_set("@names_to_ids", names_to_ids)
+      end
+      names_to_ids[name]
     end
 
     def struct_fields

Modified: incubator/thrift/trunk/lib/rb/spec/struct_spec.rb
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/rb/spec/struct_spec.rb?rev=688891&r1=688890&r2=688891&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/rb/spec/struct_spec.rb (original)
+++ incubator/thrift/trunk/lib/rb/spec/struct_spec.rb Mon Aug 25 14:34:19 2008
@@ -197,8 +197,7 @@
     end
 
     it "should raise an exception when unknown types are given to Thrift::Struct.new" do
-      lambda { Hello.new(:fish => 'salmon') }.should raise_error(Exception, "Unknown keys given to SpecNamespace::Hello.new: fish")
-      lambda { Hello.new(:foo => 'bar', :baz => 'blah', :greeting => 'Good day') }.should raise_error(Exception, /^Unknown keys given to SpecNamespace::Hello.new: (foo, baz|baz, foo)$/)
+      lambda { Hello.new(:fish => 'salmon') }.should raise_error(Exception, "Unknown key given to SpecNamespace::Hello.new: fish")
     end
 
     it "should support `raise Xception, 'message'` for Exception structs" do