Merge pull request #7 from stitchfix/fix-to_h

to_h will only call methods with zero args

lib/immutable-struct.rb

@@ -22,7 +22,7 @@ class ImmutableStruct
   #
   # Example:
   #
-  #     Person = ImmutableStruct.new(:name, :location, :minor?)
+  #     Person = ImmutableStruct.new(:name, :location, :minor?, [:aliases])
   #
   #     p = Person.new(name: 'Dave', location: Location.new("DC"), minor: false)
   #     p.name     # => 'Dave'
@@ -41,6 +41,21 @@ class ImmutableStruct
   #     new_person.age     # => 41
   #     new_person.active? # => true
   #
+  # Note that you also get an implementation of `to_h` that will include **all** no-arg methods in its
+  # output:
+  #
+  #     Person = ImmutableStruct.new(:name, :location, :minor?, [:aliases])
+  #     p = Person.new(name: 'Dave', minor: "yup", aliases: [ "davetron", "davetron5000" ])
+  #     p.to_h # => { name: "Dave", minor: "yup", minor?: true, aliases: ["davetron", "davetron5000" ] }
+  # 
+  # This has two subtle side-effects:
+  #
+  # * Methods that take no args, but are not 'attributes' will get called by `to_h`.  This shouldn't be a
+  #   problem, because you should not generally be doing this on a struct-like class.
+  # * Methods that take no args, but call `to_h` will stack overflow.  This is because the class'
+  #   internals have no way to know about this.  This is particularly a problem if you want to
+  #   define your own `to_json` method that serializes the result of `to_h`.
+  #
   def self.new(*attributes,&block)
     klass = Class.new do
       attributes.each do |attribute|
@@ -89,7 +104,13 @@ def self.new(*attributes,&block)
       end
     end
     klass.class_exec(&block) unless block.nil?
-    imethods = klass.instance_methods(include_super=false)
+
+    imethods = klass.instance_methods(include_super=false).map { |method_name|
+      klass.instance_method(method_name)
+    }.reject { |method|
+      method.arity != 0
+    }.map(&:name).map(&:to_sym)
+
     klass.class_exec(imethods) do |imethods|
       define_method(:to_h) do
         imethods.inject({}) do |hash, method|

spec/immutable_struct_spec.rb

@@ -1,4 +1,5 @@
 require 'spec_helper.rb'
+require 'json'
 
 module TestModule
   def hello; "hello"; end
@@ -112,14 +113,61 @@ def self.from_array(array)
   end
 
   describe "to_h" do
-    it "should include the output of params and block methods in the hash" do
-      klass = ImmutableStruct.new(:flappy) do
-        def lawsuit
-          'pending'
+    context "vanilla struct with just derived values" do
+      it "should include the output of params and block methods in the hash" do
+        klass = ImmutableStruct.new(:name, :minor?, :location, [:aliases]) do
+          def nick_name
+            'bob'
+          end
         end
+        instance = klass.new(name: "Rudy", minor: "ayup", aliases: [ "Rudyard", "Roozoola" ])
+        instance.to_h.should == {
+          name: "Rudy",
+          minor: "ayup",
+          minor?: true,
+          location: nil,
+          aliases: [ "Rudyard", "Roozoola"],
+          nick_name: "bob",
+        }
+      end
+    end
+    context "additional method that takes arguments" do
+      it "should not call the additional method" do
+        klass = ImmutableStruct.new(:name, :minor?, :location, [:aliases]) do
+          def nick_name
+            'bob'
+          end
+          def location_near?(other_location)
+            false
+          end
+        end
+        instance = klass.new(name: "Rudy", minor: "ayup", aliases: [ "Rudyard", "Roozoola" ])
+        instance.to_h.should == {
+          name: "Rudy",
+          minor: "ayup",
+          minor?: true,
+          location: nil,
+          aliases: [ "Rudyard", "Roozoola"],
+          nick_name: "bob",
+        }
+      end
+    end
+
+    context "no-arg method that uses to_h" do
+      it "blows up" do
+        klass = ImmutableStruct.new(:name, :minor?, :location, [:aliases]) do
+          def nick_name
+            'bob'
+          end
+          def to_json
+            to_h.to_json
+          end
+        end
+        instance = klass.new(name: "Rudy", minor: "ayup", aliases: [ "Rudyard", "Roozoola" ])
+        expect {
+          instance.to_json.should == instance.to_h.to_json
+        }.to raise_error(SystemStackError)
       end
-      instance = klass.new(flappy: 'bird')
-      instance.to_h.should == {flappy: 'bird', lawsuit: 'pending'}
     end
   end