Merge pull request #1 from tinkerbox/enhancement/suggestions

Some minor suggestions

Author: tbsmatt

lib/value_objects/valid_validator.rb

@@ -4,7 +4,7 @@ module ValueObjects
   class ValidValidator < ActiveModel::EachValidator
 
     def validate_each(record, attribute, value)
-      record.errors.add(attribute, :invalid) unless value && (value.is_a?(Array) ? value.count(&:invalid?) == 0 : value.valid?)
+      record.errors.add(attribute, :invalid) unless value && Array(value).count(&:invalid?) == 0
     end
 
   end

lib/value_objects/value.rb

@@ -7,11 +7,11 @@ class Value
     include ::ActiveModel::Validations
 
     def ==(other)
-      self.class == other.class && self.class.attrs.all? { |key| send(key) == other.send(key) }
+      self.class == other.class && self.class.attrs.all? { |key| public_send(key) == other.public_send(key) }
     end
 
     def to_hash
-      self.class.attrs.each_with_object({}) { |key, hash| hash[key] = send(key) }
+      self.class.attrs.each_with_object({}) { |key, hash| hash[key] = public_send(key) }
     end
 
     class << self
@@ -51,9 +51,7 @@ def new(attributes)
           # Data encoded with the 'application/x-www-form-urlencoded' media type cannot represent empty collections.
           # As a workaround, a dummy item can be added to the collection with it's key set to '-1'.
           # This dummy item will be ignored when initializing the value collection.
-          values = []
-          attributes.each { |k, v| values << @value_class.new(v) if k != '-1' }
-          values
+          attributes.map { |k, v| @value_class.new(v) if k != '-1' }.compact
         end
 
         def load(values)

spec/value_objects/valid_validator_spec.rb

@@ -22,6 +22,9 @@ class NilForbiddenRecord
 
   end
 
+  let(:valid_value) { instance_double('ValidValue', invalid?: false) }
+  let(:invalid_value) { instance_double('InvalidValue', invalid?: true) }
+
   context 'with allow_nil' do
 
     let(:record) { NilAllowedRecord.new(foo: value) }
@@ -41,7 +44,7 @@ class NilForbiddenRecord
 
     context 'and invalid value' do
 
-      let(:value) { instance_double('InvalidValue', valid?: false) }
+      let(:value) { invalid_value }
 
       it 'fails validation' do
         aggregate_failures do
@@ -54,7 +57,7 @@ class NilForbiddenRecord
 
     context 'and valid value' do
 
-      let(:value) { instance_double('ValidValue', valid?: true) }
+      let(:value) { valid_value }
 
       it 'passes validation' do
         aggregate_failures do
@@ -87,7 +90,7 @@ class NilForbiddenRecord
 
     context 'and invalid value' do
 
-      let(:value) { instance_double('InvalidValue', valid?: false) }
+      let(:value) { invalid_value }
 
       it 'fails validation' do
         aggregate_failures do
@@ -100,7 +103,7 @@ class NilForbiddenRecord
 
     context 'and valid value' do
 
-      let(:value) { instance_double('ValidValue', valid?: true) }
+      let(:value) { valid_value }
 
       it 'passes validation' do
         aggregate_failures do
@@ -133,8 +136,8 @@ class NilForbiddenRecord
     context 'and no invalid values' do
 
       let(:values) { [value1, value2] }
-      let(:value1) { instance_double('ValidValue') }
-      let(:value2) { instance_double('ValidValue') }
+      let(:value1) { valid_value }
+      let(:value2) { valid_value }
 
       it 'passes validation' do
         expect(value1).to receive(:invalid?).and_return(false).once
@@ -149,13 +152,15 @@ class NilForbiddenRecord
 
     context 'and some invalid values' do
 
-      let(:values) { [value1, value2] }
-      let(:value1) { instance_double('ValidValue') }
-      let(:value2) { instance_double('InvalidValue') }
+      let(:values) { [value1, value2, value3] }
+      let(:value1) { invalid_value }
+      let(:value2) { valid_value }
+      let(:value3) { invalid_value }
 
       it 'fails validation' do
-        expect(value1).to receive(:invalid?).and_return(false).once
-        expect(value2).to receive(:invalid?).and_return(true).once
+        expect(value1).to receive(:invalid?).and_return(true).once
+        expect(value2).to receive(:invalid?).and_return(false).once
+        expect(value3).to receive(:invalid?).and_return(true).once
         aggregate_failures do
           expect(record.valid?).to eq(false)
           expect(record.errors).to include(:foo)